Skip to content

Commit

Permalink
fix(sql): pivot joining of m:n when target entity is null
Browse files Browse the repository at this point in the history
Closes #548
  • Loading branch information
B4nan committed Aug 9, 2020
1 parent 7928c50 commit 3b05a59
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 7 deletions.
6 changes: 3 additions & 3 deletions packages/knex/src/query/CriteriaNode.ts
Expand Up @@ -61,8 +61,8 @@ export class CriteriaNode {
const type = this.prop ? this.prop.reference : null;
const composite = this.prop && this.prop.joinColumns ? this.prop.joinColumns.length > 1 : false;
const customExpression = QueryBuilderHelper.isCustomExpression(this.key!);
const scalar = Utils.isPrimaryKey(payload) || payload instanceof RegExp || payload instanceof Date || customExpression;
const operator = Utils.isObject(payload) && Object.keys(payload).every(k => Utils.isOperator(k, false));
const scalar = payload === null || Utils.isPrimaryKey(payload) || payload instanceof RegExp || payload instanceof Date || customExpression;
const operator = Utils.isPlainObject(payload) && Object.keys(payload).every(k => Utils.isOperator(k, false));

if (composite) {
return true;
Expand Down Expand Up @@ -116,7 +116,7 @@ export class CriteriaNode {
}

const customExpression = QueryBuilderHelper.isCustomExpression(this.key);
const scalar = Utils.isPrimaryKey(this.payload) || this.payload instanceof RegExp || this.payload instanceof Date || customExpression;
const scalar = this.payload === null || Utils.isPrimaryKey(this.payload) || this.payload instanceof RegExp || this.payload instanceof Date || customExpression;
const operator = Utils.isObject(this.payload) && Object.keys(this.payload).every(k => Utils.isOperator(k, false));
const pivotJoin = this.prop.reference === ReferenceType.MANY_TO_MANY && (scalar || operator);

Expand Down
5 changes: 2 additions & 3 deletions packages/knex/src/query/ObjectCriteriaNode.ts
Expand Up @@ -97,17 +97,16 @@ export class ObjectCriteriaNode extends CriteriaNode {

const embeddable = this.prop.reference === ReferenceType.EMBEDDED;
const knownKey = [ReferenceType.SCALAR, ReferenceType.MANY_TO_ONE, ReferenceType.EMBEDDED].includes(this.prop.reference) || (this.prop.reference === ReferenceType.ONE_TO_ONE && this.prop.owner);
const composite = this.prop.joinColumns && this.prop.joinColumns.length > 1;
const operatorKeys = knownKey && Object.keys(this.payload).every(key => Utils.isOperator(key, false));

return !nestedAlias && !operatorKeys && !composite && !embeddable;
return !nestedAlias && !operatorKeys && !embeddable;
}

private autoJoin(qb: QueryBuilder, alias: string): string {
const nestedAlias = qb.getNextAlias();
const customExpression = QueryBuilderHelper.isCustomExpression(this.key!);
const scalar = Utils.isPrimaryKey(this.payload) || this.payload instanceof RegExp || this.payload instanceof Date || customExpression;
const operator = Utils.isObject(this.payload) && Object.keys(this.payload).every(k => Utils.isOperator(k, false));
const operator = Utils.isPlainObject(this.payload) && Object.keys(this.payload).every(k => Utils.isOperator(k, false));
const field = `${alias}.${this.prop!.name}`;

if (this.prop!.reference === ReferenceType.MANY_TO_MANY && (scalar || operator)) {
Expand Down
35 changes: 34 additions & 1 deletion tests/QueryBuilder.test.ts
Expand Up @@ -77,7 +77,7 @@ describe('QueryBuilder', () => {
test('select query with auto-joined composite key entity', async () => {
const qb1 = orm.em.createQueryBuilder(CarOwner2);
qb1.select('*').where({ car: { name: 'Audi A8', year: 2010 } });
expect(qb1.getQuery()).toEqual('select `e0`.* from `car_owner2` as `e0` where `e0`.`name` = ? and `e0`.`year` = ?');
expect(qb1.getQuery()).toEqual('select `e0`.* from `car_owner2` as `e0` left join `car2` as `e1` on `e0`.`car_name` = `e1`.`name` and `e0`.`car_year` = `e1`.`year` where `e1`.`name` = ? and `e1`.`year` = ?');
expect(qb1.getParams()).toEqual(['Audi A8', 2010]);

const qb2 = orm.em.createQueryBuilder(CarOwner2);
Expand Down Expand Up @@ -1295,6 +1295,39 @@ describe('QueryBuilder', () => {
expect(qb.getAliasForJoinPath(node.getPath())).toBe('a');
});

test('pivot joining of m:n when target entity is null (GH issue 548)', async () => {
const qb11 = await orm.em.createQueryBuilder(User2, 'u').select('u.*').where({ cars: null });
expect(qb11.getQuery()).toMatch('select `u`.* ' +
'from `user2` as `u` ' +
'left join `user2_cars` as `e1` on `u`.`first_name` = `e1`.`user2_first_name` and `u`.`last_name` = `e1`.`user2_last_name` ' +
'where (`e1`.`car2_name`, `e1`.`car2_year`) is null');
expect(qb11.getParams()).toEqual([]);

const qb2 = await orm.em.createQueryBuilder(Book2, 'b').select('b.*').where({ $or: [{ tags: null }, { tags: { $ne: 1 } }] });
expect(qb2.getQuery()).toMatch('select `b`.*, `b`.price * 1.19 as `price_taxed` ' +
'from `book2` as `b` ' +
'left join `book2_tags` as `e1` on `b`.`uuid_pk` = `e1`.`book2_uuid_pk` ' +
'where (`e1`.`book_tag2_id` is null or `e1`.`book_tag2_id` != ?)');
expect(qb2.getParams()).toEqual([1]);

const qb3 = await orm.em.createQueryBuilder(Author2, 'a').select('a.*').where({ friends: null }).orderBy({ friends: { name: QueryOrder.ASC } });
expect(qb3.getQuery()).toMatch('select `a`.* ' +
'from `author2` as `a` ' +
'left join `author_to_friend` as `e1` on `a`.`id` = `e1`.`author2_1_id` ' +
'left join `author2` as `e2` on `e1`.`author2_2_id` = `e2`.`id` ' +
'where `e1`.`author2_2_id` is null ' +
'order by `e2`.`name` asc');
expect(qb3.getParams()).toEqual([]);

const qb4 = await orm.em.createQueryBuilder(Author2, 'a').select('a.*').where({ friends: null }).orderBy({ friends: QueryOrder.ASC });
expect(qb4.getQuery()).toMatch('select `a`.* ' +
'from `author2` as `a` ' +
'left join `author_to_friend` as `e1` on `a`.`id` = `e1`.`author2_1_id` ' +
'where `e1`.`author2_2_id` is null ' +
'order by `e1`.`author2_2_id` asc');
expect(qb4.getParams()).toEqual([]);
});

afterAll(async () => orm.close(true));

});

0 comments on commit 3b05a59

Please sign in to comment.