Skip to content

Commit b62fb05

Browse files
B4nanB4nan
committed
fix(query-builder): support joining same property multiple times
Closes #2602
1 parent 38599da commit b62fb05

File tree

3 files changed

+46
-13
lines changed

3 files changed

+46
-13
lines changed

‎packages/knex/src/query/QueryBuilder.ts‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -595,7 +595,7 @@ export class QueryBuilder<T extends AnyEntity<T> = AnyEntity> {
595595

596596
this._aliasMap[alias] = prop.type;
597597
cond = QueryHelper.processWhere(cond, this.entityName, this.metadata, this.platform)!;
598-
const aliasedName = `${fromAlias}.${prop.name}`;
598+
let aliasedName = `${fromAlias}.${prop.name}#${alias}`;
599599
path ??= `${(Object.values(this._joins).find(j => j.alias === fromAlias)?.path ?? entityName)}.${prop.name}`;
600600

601601
if (prop.reference === ReferenceType.ONE_TO_MANY) {
@@ -606,6 +606,7 @@ export class QueryBuilder<T extends AnyEntity<T> = AnyEntity> {
606606
if (type !== 'pivotJoin') {
607607
const oldPivotAlias = this.getAliasForJoinPath(path + '[pivot]');
608608
pivotAlias = oldPivotAlias ?? this.getNextAlias(prop.pivotTable);
609+
aliasedName = `${fromAlias}.${prop.name}#${pivotAlias}`;
609610
}
610611

611612
const joins = this.helper.joinManyToManyReference(prop, fromAlias, alias, pivotAlias, type, cond, path);
@@ -632,8 +633,10 @@ export class QueryBuilder<T extends AnyEntity<T> = AnyEntity> {
632633
return ret.push(f);
633634
}
634635

635-
if (this._joins[f] && type === 'where') {
636-
return ret.push(...this.helper.mapJoinColumns(this.type, this._joins[f]) as string[]);
636+
const join = Object.keys(this._joins).find(k => f === k.substring(0, k.indexOf('#')))!;
637+
638+
if (join && type === 'where') {
639+
return ret.push(...this.helper.mapJoinColumns(this.type, this._joins[join]) as string[]);
637640
}
638641

639642
ret.push(this.helper.mapper(f, this.type) as string);
@@ -648,7 +651,7 @@ export class QueryBuilder<T extends AnyEntity<T> = AnyEntity> {
648651
}
649652

650653
Object.keys(this._populateMap).forEach(f => {
651-
if (!fields.includes(f) && type === 'where') {
654+
if (!fields.includes(f.replace(/#\w+$/, '')) && type === 'where') {
652655
ret.push(...this.helper.mapJoinColumns(this.type, this._joins[f]) as string[]);
653656
}
654657

@@ -758,17 +761,19 @@ export class QueryBuilder<T extends AnyEntity<T> = AnyEntity> {
758761
this._populate.forEach(({ field }) => {
759762
const [fromAlias, fromField] = this.helper.splitField(field);
760763
const aliasedField = `${fromAlias}.${fromField}`;
764+
const join = Object.keys(this._joins).find(k => `${aliasedField}#${this._joins[k].alias}` === k)!;
761765

762-
if (this._joins[aliasedField] && this.helper.isOneToOneInverse(field)) {
763-
return this._populateMap[aliasedField] = this._joins[aliasedField].alias;
766+
if (this._joins[join] && this.helper.isOneToOneInverse(fromField)) {
767+
return this._populateMap[join] = this._joins[join].alias;
764768
}
765769

766770
if (this.metadata.find(field)?.pivotTable) { // pivot table entity
767771
this.autoJoinPivotTable(field);
768-
} else if (meta && this.helper.isOneToOneInverse(field)) {
769-
const prop = meta.properties[field];
770-
this._joins[prop.name] = this.helper.joinOneToReference(prop, this.alias, this.getNextAlias(prop.pivotTable ?? prop.type), 'leftJoin');
771-
this._populateMap[field] = this._joins[field].alias;
772+
} else if (meta && this.helper.isOneToOneInverse(fromField)) {
773+
const prop = meta.properties[fromField];
774+
const alias = this.getNextAlias(prop.pivotTable ?? prop.type);
775+
this._joins[join] = this.helper.joinOneToReference(prop, this.alias, alias, 'leftJoin');
776+
this._populateMap[join] = this._joins[join].alias;
772777
}
773778
});
774779

‎packages/knex/src/query/QueryBuilderHelper.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ export class QueryBuilderHelper {
149149

150150
joinManyToManyReference(prop: EntityProperty, ownerAlias: string, alias: string, pivotAlias: string, type: 'leftJoin' | 'innerJoin' | 'pivotJoin', cond: Dictionary, path: string): Dictionary<JoinOptions> {
151151
const ret = {
152-
[`${ownerAlias}.${prop.name}`]: {
152+
[`${ownerAlias}.${prop.name}#${pivotAlias}`]: {
153153
prop, type, cond, ownerAlias,
154154
alias: pivotAlias,
155155
inverseAlias: alias,
@@ -166,8 +166,8 @@ export class QueryBuilderHelper {
166166
}
167167

168168
const prop2 = this.metadata.find(prop.pivotTable)!.properties[prop.type + (prop.owner ? '_inverse' : '_owner')];
169-
ret[`${pivotAlias}.${prop2.name}`] = this.joinManyToOneReference(prop2, pivotAlias, alias, type);
170-
ret[`${pivotAlias}.${prop2.name}`].path = path;
169+
ret[`${pivotAlias}.${prop2.name}#${alias}`] = this.joinManyToOneReference(prop2, pivotAlias, alias, type);
170+
ret[`${pivotAlias}.${prop2.name}#${alias}`].path = path;
171171

172172
return ret;
173173
}

‎tests/QueryBuilder.test.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,23 @@ describe('QueryBuilder', () => {
439439
expect(qb.getParams()).toEqual(['test 123', '%3', 2, 1]);
440440
});
441441

442+
test('select with leftJoin for same property multiple times', async () => {
443+
const qb = orm.em.createQueryBuilder(Publisher2, 'p');
444+
qb.select(['p.*', 'b.*', 'b2.*'])
445+
.leftJoin('books', 'b')
446+
.leftJoin('books', 'b2')
447+
.where({ 'b.title': 'test 123', 'b2.title': /3$/ })
448+
.limit(2, 1);
449+
const sql = 'select `p`.*, `b`.*, `b2`.* ' +
450+
'from `publisher2` as `p` ' +
451+
'left join `book2` as `b` on `p`.`id` = `b`.`publisher_id` ' +
452+
'left join `book2` as `b2` on `p`.`id` = `b2`.`publisher_id` ' +
453+
'where `b`.`title` = ? and `b2`.`title` like ? ' +
454+
'limit ? offset ?';
455+
expect(qb.getQuery()).toEqual(sql);
456+
expect(qb.getParams()).toEqual(['test 123', '%3', 2, 1]);
457+
});
458+
442459
test('select with boolean', async () => {
443460
const qb = orm.em.createQueryBuilder(Author2);
444461
qb.select('*').where({ termsAccepted: false });
@@ -1831,6 +1848,17 @@ describe('QueryBuilder', () => {
18311848
expect(qb4.getParams()).toEqual([]);
18321849
});
18331850

1851+
test('123 pivot joining of m:n when no target entity needed directly (GH issue 549)', async () => {
1852+
const qb3 = orm.em.createQueryBuilder(Author2, 'a').select('a.*').where({ friends: null }).orderBy({ friends: { name: QueryOrder.ASC } });
1853+
expect(qb3.getQuery()).toMatch('select `a`.* ' +
1854+
'from `author2` as `a` ' +
1855+
'left join `author_to_friend` as `e1` on `a`.`id` = `e1`.`author2_1_id` ' +
1856+
'left join `author2` as `e2` on `e1`.`author2_2_id` = `e2`.`id` ' +
1857+
'where `e1`.`author2_2_id` is null ' +
1858+
'order by `e2`.`name` asc');
1859+
expect(qb3.getParams()).toEqual([]);
1860+
});
1861+
18341862
test('pivot joining of m:n when no target entity needed directly (GH issue 549)', async () => {
18351863
const qb1 = orm.em.createQueryBuilder(Book2, 'b').select('b.*').where({ tags: { id: 1 } });
18361864
expect(qb1.getQuery()).toMatch('select `b`.*, `b`.price * 1.19 as `price_taxed` ' +

0 commit comments

Comments
 (0)