Skip to content

Commit

Permalink
fix(query-builder): ensure inner paginate query selects sub-queries u…
Browse files Browse the repository at this point in the history
…sed in orderBy

Closes #4104
  • Loading branch information
B4nan committed Mar 12, 2023
1 parent f2fa2bd commit 22b7146
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 4 deletions.
37 changes: 33 additions & 4 deletions packages/knex/src/query/QueryBuilder.ts
Expand Up @@ -730,7 +730,12 @@ export class QueryBuilder<T extends object = AnyEntity> {
alias = meta?.properties[f]?.fieldNames[0] ?? alias;
}

return qb.as(alias);
const ret = qb.as(alias);

// tag the instance, so it is possible to detect it easily
Object.defineProperty(ret, '__as', { enumerable: false, value: alias });

return ret;
}

clone(): QueryBuilder<T> {
Expand Down Expand Up @@ -1066,16 +1071,23 @@ export class QueryBuilder<T extends object = AnyEntity> {
subQuery.offset(this._offset);
}

const addToSelect = [];

if (this._orderBy.length > 0) {
const orderBy = [];

for (const orderMap of this._orderBy) {
for (const [field, direction] of Object.entries(orderMap)) {
const [a, f] = this.helper.splitField(field);
const prop = this.helper.getProperty(f, a);
const type = this.platform.castColumn(prop);
orderBy.push({
[`min(${this.ref(this.helper.mapper(field, this.type, undefined, null))}${type})`]: direction,
});
const fieldName = this.helper.mapper(field, this.type, undefined, null);

if (!prop?.persist && !prop?.formula) {
addToSelect.push(fieldName);
}

orderBy.push({ [`min(${this.ref(fieldName)}${type})`]: direction });
}
}

Expand All @@ -1085,6 +1097,23 @@ export class QueryBuilder<T extends object = AnyEntity> {
subQuery.finalized = true;
const knexQuery = subQuery.as(this.mainAlias.aliasName).clearSelect().select(pks);

if (addToSelect.length > 0) {
addToSelect.forEach(prop => {
const field = this._fields!.find(field => {
if (typeof field === 'object' && field && '__as' in field) {
return field.__as === prop;
}

// not perfect, but should work most of the time, ideally we should check only the alias (`... as alias`)
return field.toString().includes(prop);
});

if (field) {
knexQuery.select(field as string);
}
});
}

// multiple sub-queries are needed to get around mysql limitations with order by + limit + where in + group by (o.O)
// https://stackoverflow.com/questions/17892762/mysql-this-version-of-mysql-doesnt-yet-support-limit-in-all-any-some-subqu
const subSubQuery = this.getKnex().select(pks).from(knexQuery);
Expand Down
13 changes: 13 additions & 0 deletions tests/QueryBuilder.test.ts
Expand Up @@ -657,6 +657,19 @@ describe('QueryBuilder', () => {
expect(qb2.getParams()).toEqual([]);
});

test('GH #4104', async () => {
const qb = orm.em.createQueryBuilder(Author2, 'a');
const qb1 = orm.em.createQueryBuilder(Book2, 'b').count('b.uuid', true).where({ author: qb.ref('a.id') }).as('Author2.booksTotal');
qb.select(['*', qb1])
.where({ books: { title: 'foo' } })
.limit(1)
.orderBy({ booksTotal: QueryOrder.ASC });

await qb;
expect(qb.getQuery()).toEqual('select `a`.*, (select count(distinct `b`.`uuid_pk`) as `count` from `book2` as `b` where `b`.`author_id` = `a`.`id`) as `books_total` from `author2` as `a` left join `book2` as `e1` on `a`.`id` = `e1`.`author_id` where `a`.`id` in (select `a`.`id` from (select `a`.`id`, (select count(distinct `b`.`uuid_pk`) as `count` from `book2` as `b` where `b`.`author_id` = `a`.`id`) as `books_total` from `author2` as `a` left join `book2` as `e1` on `a`.`id` = `e1`.`author_id` where `e1`.`title` = ? group by `a`.`id` order by min(`books_total`) asc limit ?) as `a`) order by `books_total` asc');
expect(qb.getParams()).toEqual(['foo', 1]);
});

test('select by 1:m', async () => {
const qb = orm.em.createQueryBuilder(Author2);
qb.select('*').where({ books: { $in: ['123', '321'] } });
Expand Down

0 comments on commit 22b7146

Please sign in to comment.