From f5a6374f6e22470f63ef6257f7271c818ed09321 Mon Sep 17 00:00:00 2001 From: Doug Martin Date: Mon, 15 Mar 2021 23:23:47 -0600 Subject: [PATCH] fix(typeorm, #954): Filtering on relations with pagination (#977) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test: Test filtering on relations * test: Test relation query with overlapping relation matches and pagination (#954) * fix(typeorm, #954): use skip/take when joins are present) * docs: Note performance issues when using relation filters and pagination * style(comment): Added missing param in applyPaging comment * test: added ordering to make tests stable across different db vendors * fix(typeorm, #954): Filtering on relations with pagination * Added tests to typeorm-query.service.spec for filtering on relations with paging * Reverted basic example. Co-authored-by: Max Wölk --- documentation/docs/concepts/queries.mdx | 274 +++++++++++------- .../services/typeorm-query.service.spec.ts | 93 +++++- .../src/query/filter-query.builder.ts | 41 ++- 3 files changed, 294 insertions(+), 114 deletions(-) diff --git a/documentation/docs/concepts/queries.mdx b/documentation/docs/concepts/queries.mdx index f075f6d44..f52f9b7dd 100644 --- a/documentation/docs/concepts/queries.mdx +++ b/documentation/docs/concepts/queries.mdx @@ -6,13 +6,13 @@ import Tabs from '@theme/Tabs'; import TabItem from '@theme/TabItem'; The core of `nestjs-query` is the `Query`, it is used by `@nestjs-query/query-graphql`, `@nestjs-query/query-typeorm` - `@nestjs-query/query-sequelize` and `@nestjs-query/query-mongoose`. +`@nestjs-query/query-sequelize` and `@nestjs-query/query-mongoose`. The query interface contains three optional fields. -* `filter` -* `paging` -* `sorting` +- `filter` +- `paging` +- `sorting` All examples will be based on the following class. @@ -43,7 +43,7 @@ import { Query } from '@nestjs-query/core'; const q: Query = { filter: { - title: {eq: 'Foo Bar'}, + title: { eq: 'Foo Bar' }, }, }; ``` @@ -58,9 +58,9 @@ import { Query } from '@nestjs-query/core'; const q: Query = { filter: { // title = 'Foo Bar' AND completed IS TRUE and age > 10 - title: {eq: 'Foo Bar'}, + title: { eq: 'Foo Bar' }, completed: { is: true }, - age: {gt: 10}, + age: { gt: 10 }, }, }; ``` @@ -75,7 +75,7 @@ import { Query } from '@nestjs-query/core'; const q: Query = { filter: { // title = 'Foo Bar' OR field LIKE '%foo%' - title: {eq: 'Foo Bar', like: '%foo%'}, + title: { eq: 'Foo Bar', like: '%foo%' }, }, }; ``` @@ -90,10 +90,7 @@ In this example we `AND` two filters for the same property together: `age >= 10 ```ts const q: Query = { filter: { - and: [ - { age: { gte: 10 } }, - { age: { lte: 20 } }, - ], + and: [{ age: { gte: 10 } }, { age: { lte: 20 } }], }, }; ``` @@ -103,10 +100,7 @@ In this example a simple `OR` condition is created: `age >= 10 OR title NOT LIKE ```ts const q: Query = { filter: { - or: [ - { age: { gte: 10 } }, - { title: { notLike: '%bar' } }, - ], + or: [{ age: { gte: 10 } }, { title: { notLike: '%bar' } }], }, }; ``` @@ -119,10 +113,7 @@ const q: Query = { and: [ { age: { gte: 10 } }, { - or: [ - { title: { like: '%bar' } }, - { title: { eq: 'foobar' } }, - ], + or: [{ title: { like: '%bar' } }, { title: { eq: 'foobar' } }], }, ], }, @@ -179,6 +170,15 @@ const q: Query = { +:::note +When using filters on relations with `typeorm` in combination with paging, performance can be degraded on large result +sets. For more info see this [issue](https://github.com/doug-martin/nestjs-query/issues/954) + +In short two queries will be executed: +* The first one fetching a distinct list of primary keys with paging applied. +* The second uses primary keys from the first query to fetch the actual records. +::: + --- ## Sorting @@ -186,9 +186,10 @@ const q: Query = { The `sorting` field allows to specify the sort order for your query. The `sorting` field is an array of object containing: - * `field` - the field to sort on - * `direction` - `ASC` or `DESC` - * `nulls?` - Optional nulls sort, `NULLS_FIRST` or `NULLS_LAST` + +- `field` - the field to sort on +- `direction` - `ASC` or `DESC` +- `nulls?` - Optional nulls sort, `NULLS_FIRST` or `NULLS_LAST` = { - sorting: [{field: 'title', direction: SortDirection.DESC}], + sorting: [{ field: 'title', direction: SortDirection.DESC }], }; ``` @@ -214,12 +215,11 @@ const q: Query = { // import { SortDirection } from '@nestjs-query/core'; const q: Query = { - sorting: [ - {field: 'title', direction: SortDirection.DESC}, - {field: 'age', direction: SortDirection.ASC}, - ], + sorting: [ + { field: 'title', direction: SortDirection.DESC }, + { field: 'age', direction: SortDirection.ASC }, + ], }; - ``` @@ -237,85 +237,143 @@ The following examples show an approximation of the SQL that will be generated. All types support the following comparisons. -* `is` - Check is a field is `null`, `true` or `false`. - ```ts - // title IS NULL - { title: { is: null } } - // completed IS TRUE - { completed: { is: true } } - // completed IS false - { completed: { is: false } } - ``` -* `isNot` - Check is a field is not `null`, `true` or `false`. - ```ts - // title IS NOT NULL - { title: { isNot: null } } - // completed IS NOT TRUE - { completed: { isNot: true } } - // completed IS NOT false - { completed: { isNot: false } } - ``` -* `neq` - field is not equal to a value. - ```ts - // title != 'foo' - { title: { neq: 'foo' } } - ``` -* `gt` - field is greater than a value. - ```ts - // title > 'foo' - { title: { gt: 'foo' } } - ``` -* `gte` - field is greater than or equal to a value. - ```ts - // title >= 'foo' - { title: { gte: 'foo' } } - ``` -* `lt` - field is less than a value. - ```ts - // title < 'foo' - { title: { lt: 'foo' } } - ``` -* `lte` - field is less than or equal to a value. - ```ts - // title <= 'foo' - { title: { lte: 'foo' } } - ``` -* `in` - field is in a list of values. - ```ts - // title IN ('foo', 'bar', 'baz') - { title: { in: ['foo', 'bar', 'baz'] } } - ``` -* `notIn` - field is not in a list of values. - ```ts - // title NOT IN ('foo', 'bar', 'baz') - { title: { notIn: ['foo', 'bar', 'baz'] } } - ``` +- `is` - Check is a field is `null`, `true` or `false`. + ```ts + // title IS NULL + { + title: { + is: null; + } + } + // completed IS TRUE + { + completed: { + is: true; + } + } + // completed IS false + { + completed: { + is: false; + } + } + ``` +- `isNot` - Check is a field is not `null`, `true` or `false`. + ```ts + // title IS NOT NULL + { + title: { + isNot: null; + } + } + // completed IS NOT TRUE + { + completed: { + isNot: true; + } + } + // completed IS NOT false + { + completed: { + isNot: false; + } + } + ``` +- `neq` - field is not equal to a value. + ```ts + // title != 'foo' + { + title: { + neq: 'foo'; + } + } + ``` +- `gt` - field is greater than a value. + ```ts + // title > 'foo' + { + title: { + gt: 'foo'; + } + } + ``` +- `gte` - field is greater than or equal to a value. + ```ts + // title >= 'foo' + { + title: { + gte: 'foo'; + } + } + ``` +- `lt` - field is less than a value. + ```ts + // title < 'foo' + { + title: { + lt: 'foo'; + } + } + ``` +- `lte` - field is less than or equal to a value. + ```ts + // title <= 'foo' + { + title: { + lte: 'foo'; + } + } + ``` +- `in` - field is in a list of values. + ```ts + // title IN ('foo', 'bar', 'baz') + { title: { in: ['foo', 'bar', 'baz'] } } + ``` +- `notIn` - field is not in a list of values. + ```ts + // title NOT IN ('foo', 'bar', 'baz') + { + title: { + notIn: ['foo', 'bar', 'baz']; + } + } + ``` ### String Comparisons -* `like` - field is like a value (case sensitive). - ```ts - // title LIKE 'Foo%' - { title: { like: 'Foo%' } } - ``` -* `notLike` - field is not like a value (case sensitive). - ```ts - // title NOT LIKE 'Foo%' - { title: { notLike: 'Foo%' } } - ``` -* `iLike` - field is like a value (case insensitive). - ```ts - // title ILIKE 'Foo%' - { title: { iLike: 'Foo%' } } - ``` -* `notILike` - field is not like a value (case insensitive). - ```ts - // title NOT ILIKE 'Foo%' - { title: { notILike: 'Foo%' } } - ``` - - - - - - +- `like` - field is like a value (case sensitive). + ```ts + // title LIKE 'Foo%' + { + title: { + like: 'Foo%'; + } + } + ``` +- `notLike` - field is not like a value (case sensitive). + ```ts + // title NOT LIKE 'Foo%' + { + title: { + notLike: 'Foo%'; + } + } + ``` +- `iLike` - field is like a value (case insensitive). + ```ts + // title ILIKE 'Foo%' + { + title: { + iLike: 'Foo%'; + } + } + ``` +- `notILike` - field is not like a value (case insensitive). + ```ts + // title NOT ILIKE 'Foo%' + { + title: { + notILike: 'Foo%'; + } + } + ``` diff --git a/packages/query-typeorm/__tests__/services/typeorm-query.service.spec.ts b/packages/query-typeorm/__tests__/services/typeorm-query.service.spec.ts index e8524bd79..b02e604b9 100644 --- a/packages/query-typeorm/__tests__/services/typeorm-query.service.spec.ts +++ b/packages/query-typeorm/__tests__/services/typeorm-query.service.spec.ts @@ -1,4 +1,4 @@ -import { Filter } from '@nestjs-query/core'; +import { Filter, SortDirection } from '@nestjs-query/core'; import { Test, TestingModule } from '@nestjs/testing'; import { plainToClass } from 'class-transformer'; import { Repository } from 'typeorm'; @@ -81,6 +81,28 @@ describe('TypeOrmQueryService', (): void => { }); expect(queryResult).toEqual([entity]); }); + + it('should allow filtering on a one to one relation with an OR clause', async () => { + const entity = TEST_ENTITIES[0]; + const queryService = moduleRef.get(TestEntityService); + const queryResult = await queryService.query({ + filter: { + or: [ + { testEntityPk: { eq: TEST_ENTITIES[1].testEntityPk } }, + { + oneTestRelation: { + testRelationPk: { + in: [`test-relations-${entity.testEntityPk}-1`, `test-relations-${entity.testEntityPk}-3`], + }, + }, + }, + ], + }, + sorting: [{ field: 'testEntityPk', direction: SortDirection.ASC }], + paging: { limit: 2 }, + }); + expect(queryResult).toEqual([entity, TEST_ENTITIES[1]]); + }); }); describe('manyToOne', () => { @@ -111,6 +133,27 @@ describe('TypeOrmQueryService', (): void => { }); expect(queryResults).toEqual(TEST_RELATIONS.slice(0, 6)); }); + + it('should allow filtering on a many to one relation with paging', async () => { + const queryService = moduleRef.get(TestRelationService); + const queryResults = await queryService.query({ + filter: { + or: [ + { testRelationPk: { eq: TEST_RELATIONS[6].testRelationPk } }, + { + testEntity: { + testEntityPk: { + in: [TEST_ENTITIES[0].testEntityPk, TEST_ENTITIES[1].testEntityPk], + }, + }, + }, + ], + }, + sorting: [{ field: 'testRelationPk', direction: SortDirection.ASC }], + paging: { limit: 3 }, + }); + expect(queryResults).toEqual(TEST_RELATIONS.slice(0, 3)); + }); }); describe('oneToMany', () => { @@ -128,6 +171,27 @@ describe('TypeOrmQueryService', (): void => { }); expect(queryResult).toEqual([entity]); }); + it('should allow filtering on a one to many relation with paging', async () => { + const entity = TEST_ENTITIES[0]; + const queryService = moduleRef.get(TestEntityService); + const queryResult = await queryService.query({ + filter: { + or: [ + { testEntityPk: { eq: TEST_ENTITIES[1].testEntityPk } }, + { + testRelations: { + testRelationPk: { + in: [`test-relations-${entity.testEntityPk}-1`, `test-relations-${entity.testEntityPk}-3`], + }, + }, + }, + ], + }, + sorting: [{ field: 'testEntityPk', direction: SortDirection.ASC }], + paging: { limit: 2 }, + }); + expect(queryResult).toEqual([entity, TEST_ENTITIES[1]]); + }); }); describe('manyToMany', () => { @@ -164,6 +228,33 @@ describe('TypeOrmQueryService', (): void => { }); expect(queryResult).toEqual([TEST_ENTITIES[2], TEST_ENTITIES[5], TEST_ENTITIES[8]]); }); + it('should allow filtering on a many to many relation with paging', async () => { + const queryService = moduleRef.get(TestEntityService); + const queryResult = await queryService.query({ + filter: { + or: [ + { testEntityPk: { eq: TEST_ENTITIES[2].testEntityPk } }, + { + manyTestRelations: { + relationName: { + in: [TEST_RELATIONS[1].relationName, TEST_RELATIONS[4].relationName], + }, + }, + }, + ], + }, + sorting: [{ field: 'numberType', direction: SortDirection.ASC }], + paging: { limit: 6 }, + }); + expect(queryResult).toEqual([ + TEST_ENTITIES[1], + TEST_ENTITIES[2], // additional one from the or + TEST_ENTITIES[3], + TEST_ENTITIES[5], + TEST_ENTITIES[7], + TEST_ENTITIES[9], + ]); + }); }); }); }); diff --git a/packages/query-typeorm/src/query/filter-query.builder.ts b/packages/query-typeorm/src/query/filter-query.builder.ts index d02dc76cc..74c933627 100644 --- a/packages/query-typeorm/src/query/filter-query.builder.ts +++ b/packages/query-typeorm/src/query/filter-query.builder.ts @@ -28,6 +28,8 @@ interface Sortable extends QueryBuilder { interface Pageable extends QueryBuilder { limit(limit?: number): this; offset(offset?: number): this; + skip(skip?: number): this; + take(take?: number): this; } /** @@ -48,21 +50,23 @@ export class FilterQueryBuilder { * @param query - the query to apply. */ select(query: Query): SelectQueryBuilder { + const hasRelations = this.filterHasRelations(query.filter); let qb = this.createQueryBuilder(); - qb = this.applyRelationJoins(qb, query.filter); + qb = hasRelations ? this.applyRelationJoins(qb, query.filter) : qb; qb = this.applyFilter(qb, query.filter, qb.alias); qb = this.applySorting(qb, query.sorting, qb.alias); - qb = this.applyPaging(qb, query.paging); + qb = this.applyPaging(qb, query.paging, hasRelations); return qb; } selectById(id: string | number | (string | number)[], query: Query): SelectQueryBuilder { + const hasRelations = this.filterHasRelations(query.filter); let qb = this.createQueryBuilder(); - qb = this.applyRelationJoins(qb, query.filter); + qb = hasRelations ? this.applyRelationJoins(qb, query.filter) : qb; qb = qb.andWhereInIds(id); qb = this.applyFilter(qb, query.filter, qb.alias); qb = this.applySorting(qb, query.sorting, qb.alias); - qb = this.applyPaging(qb, query.paging); + qb = this.applyPaging(qb, query.paging, hasRelations); return qb; } @@ -108,11 +112,17 @@ export class FilterQueryBuilder { * Applies paging to a Pageable `typeorm` query builder * @param qb - the `typeorm` QueryBuilder * @param paging - the Paging options. + * @param useSkipTake - if skip/take should be used instead of limit/offset. */ - applyPaging

>(qb: P, paging?: Paging): P { + applyPaging

>(qb: P, paging?: Paging, useSkipTake?: boolean): P { if (!paging) { return qb; } + + if (useSkipTake) { + return qb.take(paging.limit).skip(paging.offset); + } + return qb.limit(paging.limit).offset(paging.offset); } @@ -165,6 +175,13 @@ export class FilterQueryBuilder { return this.repo.createQueryBuilder(); } + /** + * Gets relations referenced in the filter and adds joins for them to the query builder + * @param qb - the `typeorm` QueryBuilder. + * @param filter - the filter. + * + * @returns the query builder for chaining + */ private applyRelationJoins(qb: SelectQueryBuilder, filter?: Filter): SelectQueryBuilder { if (!filter) { return qb; @@ -173,6 +190,20 @@ export class FilterQueryBuilder { return referencedRelations.reduce((rqb, relation) => rqb.leftJoin(`${rqb.alias}.${relation}`, relation), qb); } + /** + * Checks if a filter references any relations. + * @param filter + * @private + * + * @returns true if there are any referenced relations + */ + private filterHasRelations(filter?: Filter): boolean { + if (!filter) { + return false; + } + return this.getReferencedRelations(filter).length > 0; + } + private getReferencedRelations(filter: Filter): string[] { const { relationNames } = this; const referencedFields = getFilterFields(filter);