From 22b7146cae14a3e153ed4d144f18de1fb6b8cc45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Ad=C3=A1mek?= Date: Sun, 12 Mar 2023 18:31:32 +0100 Subject: [PATCH] fix(query-builder): ensure inner paginate query selects sub-queries used in orderBy Closes #4104 --- packages/knex/src/query/QueryBuilder.ts | 37 ++++++++++++++++++++++--- tests/QueryBuilder.test.ts | 13 +++++++++ 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/packages/knex/src/query/QueryBuilder.ts b/packages/knex/src/query/QueryBuilder.ts index 4aba99c1db74..bf7e15683e45 100644 --- a/packages/knex/src/query/QueryBuilder.ts +++ b/packages/knex/src/query/QueryBuilder.ts @@ -730,7 +730,12 @@ export class QueryBuilder { 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 { @@ -1066,16 +1071,23 @@ export class QueryBuilder { 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 }); } } @@ -1085,6 +1097,23 @@ export class QueryBuilder { 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); diff --git a/tests/QueryBuilder.test.ts b/tests/QueryBuilder.test.ts index 0883d547a9b6..d1ad73adb287 100644 --- a/tests/QueryBuilder.test.ts +++ b/tests/QueryBuilder.test.ts @@ -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'] } });