Skip to content

Commit

Permalink
fix(query-builder): fix nested ordered pagination (#2351)
Browse files Browse the repository at this point in the history
Co-authored-by: James Meneghello <james.meneghello@glxdigital.com>
  • Loading branch information
jamesmeneghello and James Meneghello committed Nov 6, 2021
1 parent 76e56fd commit c5a5c6b
Show file tree
Hide file tree
Showing 6 changed files with 340 additions and 28 deletions.
34 changes: 23 additions & 11 deletions packages/knex/src/query/QueryBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -768,23 +768,35 @@ export class QueryBuilder<T extends AnyEntity<T> = AnyEntity> {
}

private wrapPaginateSubQuery(meta: EntityMetadata): void {
const pks = this.prepareFields(meta.primaryKeys, 'sub-query');
const subQuery = this.clone().limit(undefined).offset(undefined);
subQuery.finalized = true;
const knexQuery = subQuery.as(this.alias).clearSelect().select(pks);

// 3 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).groupBy(pks).limit(this._limit!);
const pks = this.prepareFields(meta.primaryKeys, 'sub-query') as string[];
const subQuery = this.clone().select(pks).groupBy(pks).limit(this._limit!);

if (this._offset) {
subSubQuery.offset(this._offset);
subQuery.offset(this._offset);
}

const subSubSubQuery = this.getKnex().select(pks).from(subSubQuery.as(this.alias));
if (this._orderBy) {
const orderBy = [];
for (const orderMap of this._orderBy) {
for (const [field, direction] of Object.entries(orderMap)) {
orderBy.push({
[`min(${this.ref(field)})`]: direction,
});
}
}

subQuery.orderBy(orderBy);
}

subQuery.finalized = true;
const knexQuery = subQuery.as(this.alias).clearSelect().select(pks);

// 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);
this._limit = undefined;
this._offset = undefined;
this.select(this._fields!).where({ [Utils.getPrimaryKeyHash(meta.primaryKeys)]: { $in: subSubSubQuery } });
this.select(this._fields!).where({ [Utils.getPrimaryKeyHash(meta.primaryKeys)]: { $in: subSubQuery } });
}

private wrapModifySubQuery(meta: EntityMetadata): void {
Expand Down
4 changes: 1 addition & 3 deletions tests/EntityManager.mysql.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2355,12 +2355,10 @@ describe('EntityManagerMySql', () => {
'left join `book2` as `e1` on `e0`.`id` = `e1`.`author_id` ' +
'left join `address2` as `e2` on `e0`.`id` = `e2`.`author_id` where `e0`.`id` in (select `e0`.`id` ' +
'from (select `e0`.`id` ' +
'from (select `e0`.`id` ' +
'from `author2` as `e0` ' +
'left join `book2` as `e1` on `e0`.`id` = `e1`.`author_id` ' +
'left join `address2` as `e2` on `e0`.`id` = `e2`.`author_id` ' +
'where `e1`.`title` like ? order by `e0`.`name` asc, `e1`.`title` asc' +
') as `e0` group by `e0`.`id` limit ? offset ?' +
'where `e1`.`title` like ? group by `e0`.`id` order by min(`e0`.`name`) asc, min(`e1`.`title`) asc limit ? offset ?' +
') as `e0`' +
') order by `e0`.`name` asc, `e1`.`title` asc');

Expand Down
4 changes: 1 addition & 3 deletions tests/EntityManager.postgre.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1655,11 +1655,9 @@ describe('EntityManagerPostgre', () => {
'from "author2" as "e0" ' +
'left join "book2" as "e1" on "e0"."id" = "e1"."author_id" where "e0"."id" in (select "e0"."id" ' +
'from (select "e0"."id" ' +
'from (select "e0"."id" ' +
'from "author2" as "e0" ' +
'left join "book2" as "e1" on "e0"."id" = "e1"."author_id" ' +
'where "e1"."title" like $1 order by "e0"."name" asc, "e1"."title" asc' +
') as "e0" group by "e0"."id" limit $2 offset $3' +
'where "e1"."title" like $1 group by "e0"."id" order by min("e0"."name") asc, min("e1"."title") asc limit $2 offset $3' +
') as "e0"' +
') order by "e0"."name" asc, "e1"."title" asc');
});
Expand Down
10 changes: 3 additions & 7 deletions tests/QueryBuilder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -257,13 +257,11 @@ describe('QueryBuilder', () => {
'left join `foo_bar2` as `fb2` on `fz`.`id` = `fb2`.`baz_id` ' +
'where `fb1`.`id` in (' +
'select `fb1`.`id` from (' +
'select `fb1`.`id` from (' +
'select `fb1`.`id` from `foo_bar2` as `fb1` ' +
'inner join `foo_baz2` as `fz` on `fb1`.`baz_id` = `fz`.`id` ' +
'left join `foo_bar2` as `fb2` on `fz`.`id` = `fb2`.`baz_id` ' +
'where `fz`.`name` = ?) as `fb1` ' +
'group by `fb1`.`id` ' +
'limit ?) as `fb1`)';
'where `fz`.`name` = ? group by `fb1`.`id` limit ?) ' +
'as `fb1`)';
expect(qb.getQuery()).toEqual(sql);
expect(qb.getParams()).toEqual(['baz', 1]);
await qb.execute();
Expand Down Expand Up @@ -1959,11 +1957,9 @@ describe('QueryBuilder', () => {
'left join "book2_tags" as "e1" on "b"."uuid_pk" = "e1"."book2_uuid_pk" ' +
'left join "book_tag2" as "t" on "e1"."book_tag2_id" = "t"."id" where "b"."uuid_pk" in ' +
'(select "b"."uuid_pk" from ' +
'(select "b"."uuid_pk" from ' +
'(select "b"."uuid_pk" from "book2" as "b" ' +
'left join "book2_tags" as "e1" on "b"."uuid_pk" = "e1"."book2_uuid_pk" ' +
'left join "book_tag2" as "t" on "e1"."book_tag2_id" = "t"."id" where "t"."name" = $1' +
') as "b" group by "b"."uuid_pk" limit $2 offset $3' +
'left join "book_tag2" as "t" on "e1"."book_tag2_id" = "t"."id" where "t"."name" = $1 group by "b"."uuid_pk" limit $2 offset $3' +
') as "b")';
expect(qb.getQuery()).toEqual(sql);
expect(qb.getParams()).toEqual(['tag name', 20, 1]);
Expand Down
5 changes: 1 addition & 4 deletions tests/features/joined-strategy.postgre.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -364,12 +364,9 @@ describe('Joined loading strategy', () => {
'left join "foo_bar2" as "b1" on "e0"."id" = "b1"."baz_id" ' +
'where "e0"."id" in (' +
'select "e0"."id" from (' +
'select "e0"."id" from (' +
'select "e0"."id" from "foo_baz2" as "e0" ' +
'left join "foo_bar2" as "b1" on "e0"."id" = "b1"."baz_id" ' +
'where "e0"."id" = $1) as "e0" ' +
'group by "e0"."id" ' +
'limit $2 offset $3) as "e0")');
'where "e0"."id" = $1 group by "e0"."id" limit $2 offset $3) as "e0")');
});

test('nested populating', async () => {
Expand Down
Loading

0 comments on commit c5a5c6b

Please sign in to comment.