fix(query-builder): support joining same property multiple times

Closes #2602
mikro-orm · Jan 5, 2022 · b62fb05 · b62fb05
1 parent 38599da
commit b62fb05
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 13 deletions.
diff --git a/packages/knex/src/query/QueryBuilder.ts b/packages/knex/src/query/QueryBuilder.ts
@@ -595,7 +595,7 @@ export class QueryBuilder<T extends AnyEntity<T> = AnyEntity> {
 
     this._aliasMap[alias] = prop.type;
     cond = QueryHelper.processWhere(cond, this.entityName, this.metadata, this.platform)!;
-    const aliasedName = `${fromAlias}.${prop.name}`;
+    let aliasedName = `${fromAlias}.${prop.name}#${alias}`;
     path ??= `${(Object.values(this._joins).find(j => j.alias === fromAlias)?.path ?? entityName)}.${prop.name}`;
 
     if (prop.reference === ReferenceType.ONE_TO_MANY) {
@@ -606,6 +606,7 @@ export class QueryBuilder<T extends AnyEntity<T> = AnyEntity> {
       if (type !== 'pivotJoin') {
         const oldPivotAlias = this.getAliasForJoinPath(path + '[pivot]');
         pivotAlias = oldPivotAlias ?? this.getNextAlias(prop.pivotTable);
+        aliasedName = `${fromAlias}.${prop.name}#${pivotAlias}`;
       }
 
       const joins = this.helper.joinManyToManyReference(prop, fromAlias, alias, pivotAlias, type, cond, path);
@@ -632,8 +633,10 @@ export class QueryBuilder<T extends AnyEntity<T> = AnyEntity> {
         return ret.push(f);
       }
 
-      if (this._joins[f] && type === 'where') {
-        return ret.push(...this.helper.mapJoinColumns(this.type, this._joins[f]) as string[]);
+      const join = Object.keys(this._joins).find(k => f === k.substring(0, k.indexOf('#')))!;
+
+      if (join && type === 'where') {
+        return ret.push(...this.helper.mapJoinColumns(this.type, this._joins[join]) as string[]);
       }
 
       ret.push(this.helper.mapper(f, this.type) as string);
@@ -648,7 +651,7 @@ export class QueryBuilder<T extends AnyEntity<T> = AnyEntity> {
     }
 
     Object.keys(this._populateMap).forEach(f => {
-      if (!fields.includes(f) && type === 'where') {
+      if (!fields.includes(f.replace(/#\w+$/, '')) && type === 'where') {
         ret.push(...this.helper.mapJoinColumns(this.type, this._joins[f]) as string[]);
       }
 
@@ -758,17 +761,19 @@ export class QueryBuilder<T extends AnyEntity<T> = AnyEntity> {
     this._populate.forEach(({ field }) => {
       const [fromAlias, fromField] = this.helper.splitField(field);
       const aliasedField = `${fromAlias}.${fromField}`;
+      const join = Object.keys(this._joins).find(k => `${aliasedField}#${this._joins[k].alias}` === k)!;
 
-      if (this._joins[aliasedField] && this.helper.isOneToOneInverse(field)) {
-        return this._populateMap[aliasedField] = this._joins[aliasedField].alias;
+      if (this._joins[join] && this.helper.isOneToOneInverse(fromField)) {
+        return this._populateMap[join] = this._joins[join].alias;
       }
 
       if (this.metadata.find(field)?.pivotTable) { // pivot table entity
         this.autoJoinPivotTable(field);
-      } else if (meta && this.helper.isOneToOneInverse(field)) {
-        const prop = meta.properties[field];
-        this._joins[prop.name] = this.helper.joinOneToReference(prop, this.alias, this.getNextAlias(prop.pivotTable ?? prop.type), 'leftJoin');
-        this._populateMap[field] = this._joins[field].alias;
+      } else if (meta && this.helper.isOneToOneInverse(fromField)) {
+        const prop = meta.properties[fromField];
+        const alias = this.getNextAlias(prop.pivotTable ?? prop.type);
+        this._joins[join] = this.helper.joinOneToReference(prop, this.alias, alias, 'leftJoin');
+        this._populateMap[join] = this._joins[join].alias;
       }
     });
 

diff --git a/packages/knex/src/query/QueryBuilderHelper.ts b/packages/knex/src/query/QueryBuilderHelper.ts
@@ -149,7 +149,7 @@ export class QueryBuilderHelper {
 
   joinManyToManyReference(prop: EntityProperty, ownerAlias: string, alias: string, pivotAlias: string, type: 'leftJoin' | 'innerJoin' | 'pivotJoin', cond: Dictionary, path: string): Dictionary<JoinOptions> {
     const ret = {
-      [`${ownerAlias}.${prop.name}`]: {
+      [`${ownerAlias}.${prop.name}#${pivotAlias}`]: {
         prop, type, cond, ownerAlias,
         alias: pivotAlias,
         inverseAlias: alias,
@@ -166,8 +166,8 @@ export class QueryBuilderHelper {
     }
 
     const prop2 = this.metadata.find(prop.pivotTable)!.properties[prop.type + (prop.owner ? '_inverse' : '_owner')];
-    ret[`${pivotAlias}.${prop2.name}`] = this.joinManyToOneReference(prop2, pivotAlias, alias, type);
-    ret[`${pivotAlias}.${prop2.name}`].path = path;
+    ret[`${pivotAlias}.${prop2.name}#${alias}`] = this.joinManyToOneReference(prop2, pivotAlias, alias, type);
+    ret[`${pivotAlias}.${prop2.name}#${alias}`].path = path;
 
     return ret;
   }

diff --git a/tests/QueryBuilder.test.ts b/tests/QueryBuilder.test.ts
@@ -439,6 +439,23 @@ describe('QueryBuilder', () => {
     expect(qb.getParams()).toEqual(['test 123', '%3', 2, 1]);
   });
 
+  test('select with leftJoin for same property multiple times', async () => {
+    const qb = orm.em.createQueryBuilder(Publisher2, 'p');
+    qb.select(['p.*', 'b.*', 'b2.*'])
+      .leftJoin('books', 'b')
+      .leftJoin('books', 'b2')
+      .where({ 'b.title': 'test 123', 'b2.title': /3$/ })
+      .limit(2, 1);
+    const sql = 'select `p`.*, `b`.*, `b2`.* ' +
+      'from `publisher2` as `p` ' +
+      'left join `book2` as `b` on `p`.`id` = `b`.`publisher_id` ' +
+      'left join `book2` as `b2` on `p`.`id` = `b2`.`publisher_id` ' +
+      'where `b`.`title` = ? and `b2`.`title` like ? ' +
+      'limit ? offset ?';
+    expect(qb.getQuery()).toEqual(sql);
+    expect(qb.getParams()).toEqual(['test 123', '%3', 2, 1]);
+  });
+
   test('select with boolean', async () => {
     const qb = orm.em.createQueryBuilder(Author2);
     qb.select('*').where({ termsAccepted: false });
@@ -1831,6 +1848,17 @@ describe('QueryBuilder', () => {
     expect(qb4.getParams()).toEqual([]);
   });
 
+  test('123 pivot joining of m:n when no target entity needed directly (GH issue 549)', async () => {
+    const qb3 =  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([]);
+  });
+
   test('pivot joining of m:n when no target entity needed directly (GH issue 549)', async () => {
     const qb1 = orm.em.createQueryBuilder(Book2, 'b').select('b.*').where({ tags: { id: 1 } });
     expect(qb1.getQuery()).toMatch('select `b`.*, `b`.price * 1.19 as `price_taxed` ' +