diff --git a/packages/query-typeorm/__tests__/query/__snapshots__/filter-query.builder.spec.ts.snap b/packages/query-typeorm/__tests__/query/__snapshots__/filter-query.builder.spec.ts.snap index cb22f1d29..e092470ff 100644 --- a/packages/query-typeorm/__tests__/query/__snapshots__/filter-query.builder.spec.ts.snap +++ b/packages/query-typeorm/__tests__/query/__snapshots__/filter-query.builder.spec.ts.snap @@ -113,7 +113,7 @@ OFFSET 11" `; -exports[`FilterQueryBuilder #select with paging should use limit/offset when filtering on many to one relation 1`] = ` +exports[`FilterQueryBuilder #select with paging skip/take - limit/offset should use limit/offset when filtering on many to one relation 1`] = ` "SELECT "TestEntity"."test_entity_pk" AS "TestEntity_test_entity_pk", "TestEntity"."string_type" AS "TestEntity_string_type", @@ -132,7 +132,7 @@ OFFSET 3" `; -exports[`FilterQueryBuilder #select with paging should use limit/offset when filtering on one to one relation 1`] = ` +exports[`FilterQueryBuilder #select with paging skip/take - limit/offset should use limit/offset when filtering on nested many to one relation 1`] = ` "SELECT "TestEntity"."test_entity_pk" AS "TestEntity_test_entity_pk", "TestEntity"."string_type" AS "TestEntity_string_type", @@ -145,13 +145,14 @@ exports[`FilterQueryBuilder #select with paging should use limit/offset when fil FROM "test_entity" "TestEntity" LEFT JOIN "test_relation" "oneTestRelation" ON "oneTestRelation"."test_relation_pk" = "TestEntity"."oneTestRelationTestRelationPk" + LEFT JOIN "test_entity" "testEntity" ON "testEntity"."test_entity_pk" = "oneTestRelation"."test_entity_id" LIMIT 10 OFFSET 3" `; -exports[`FilterQueryBuilder #select with paging should use skip/take when filtering on one to many relation 1`] = ` +exports[`FilterQueryBuilder #select with paging skip/take - limit/offset should use limit/offset when filtering on nested one to one relation 1`] = ` "SELECT "TestEntity"."test_entity_pk" AS "TestEntity_test_entity_pk", "TestEntity"."string_type" AS "TestEntity_string_type", @@ -163,7 +164,31 @@ exports[`FilterQueryBuilder #select with paging should use skip/take when filter "TestEntity"."oneTestRelationTestRelationPk" AS "TestEntity_oneTestRelationTestRelationPk" FROM "test_entity" "TestEntity" - LEFT JOIN "test_relation" "testRelations" ON "testRelations"."test_entity_id" = "TestEntity"."test_entity_pk"" + LEFT JOIN "test_relation" "oneTestRelation" ON "oneTestRelation"."test_relation_pk" = "TestEntity"."oneTestRelationTestRelationPk" + LEFT JOIN "test_entity" "oneTestEntity" ON "oneTestEntity"."oneTestRelationTestRelationPk" = "oneTestRelation"."test_relation_pk" +LIMIT + 10 +OFFSET + 3" +`; + +exports[`FilterQueryBuilder #select with paging skip/take - limit/offset should use limit/offset when filtering on one to one relation 1`] = ` +"SELECT + "TestEntity"."test_entity_pk" AS "TestEntity_test_entity_pk", + "TestEntity"."string_type" AS "TestEntity_string_type", + "TestEntity"."bool_type" AS "TestEntity_bool_type", + "TestEntity"."number_type" AS "TestEntity_number_type", + "TestEntity"."date_type" AS "TestEntity_date_type", + "TestEntity"."many_to_one_relation_id" AS "TestEntity_many_to_one_relation_id", + "TestEntity"."many_to_one_soft_delete_relation_id" AS "TestEntity_many_to_one_soft_delete_relation_id", + "TestEntity"."oneTestRelationTestRelationPk" AS "TestEntity_oneTestRelationTestRelationPk" +FROM + "test_entity" "TestEntity" + LEFT JOIN "test_relation" "oneTestRelation" ON "oneTestRelation"."test_relation_pk" = "TestEntity"."oneTestRelationTestRelationPk" +LIMIT + 10 +OFFSET + 3" `; exports[`FilterQueryBuilder #select with paging skip/take - limit/offset should use skip/take when filtering on many to many relation 1`] = ` @@ -182,6 +207,54 @@ FROM LEFT JOIN "test_relation" "manyTestRelations" ON "manyTestRelations"."test_relation_pk" = "TestEntity_manyTestRelations"."testRelationTestRelationPk"" `; +exports[`FilterQueryBuilder #select with paging skip/take - limit/offset should use skip/take when filtering on nested many to many relation 1`] = ` +"SELECT + "TestEntity"."test_entity_pk" AS "TestEntity_test_entity_pk", + "TestEntity"."string_type" AS "TestEntity_string_type", + "TestEntity"."bool_type" AS "TestEntity_bool_type", + "TestEntity"."number_type" AS "TestEntity_number_type", + "TestEntity"."date_type" AS "TestEntity_date_type", + "TestEntity"."many_to_one_relation_id" AS "TestEntity_many_to_one_relation_id", + "TestEntity"."many_to_one_soft_delete_relation_id" AS "TestEntity_many_to_one_soft_delete_relation_id", + "TestEntity"."oneTestRelationTestRelationPk" AS "TestEntity_oneTestRelationTestRelationPk" +FROM + "test_entity" "TestEntity" + LEFT JOIN "test_relation" "oneTestRelation" ON "oneTestRelation"."test_relation_pk" = "TestEntity"."oneTestRelationTestRelationPk" + LEFT JOIN "test_entity_many_test_relations_test_relation" "manyTestEntities_oneTestRelation" ON "manyTestEntities_oneTestRelation"."testRelationTestRelationPk" = "oneTestRelation"."test_relation_pk" + LEFT JOIN "test_entity" "manyTestEntities" ON "manyTestEntities"."test_entity_pk" = "manyTestEntities_oneTestRelation"."testEntityTestEntityPk"" +`; + +exports[`FilterQueryBuilder #select with paging skip/take - limit/offset should use skip/take when filtering on nested one to many relation 1`] = ` +"SELECT + "TestEntity"."test_entity_pk" AS "TestEntity_test_entity_pk", + "TestEntity"."string_type" AS "TestEntity_string_type", + "TestEntity"."bool_type" AS "TestEntity_bool_type", + "TestEntity"."number_type" AS "TestEntity_number_type", + "TestEntity"."date_type" AS "TestEntity_date_type", + "TestEntity"."many_to_one_relation_id" AS "TestEntity_many_to_one_relation_id", + "TestEntity"."many_to_one_soft_delete_relation_id" AS "TestEntity_many_to_one_soft_delete_relation_id", + "TestEntity"."oneTestRelationTestRelationPk" AS "TestEntity_oneTestRelationTestRelationPk" +FROM + "test_entity" "TestEntity" + LEFT JOIN "test_relation" "oneTestRelation" ON "oneTestRelation"."test_relation_pk" = "TestEntity"."oneTestRelationTestRelationPk" + LEFT JOIN "test_entity_relation_entity" "testEntityRelation" ON "testEntityRelation"."test_relation_id" = "oneTestRelation"."test_relation_pk"" +`; + +exports[`FilterQueryBuilder #select with paging skip/take - limit/offset should use skip/take when filtering on one to many relation 1`] = ` +"SELECT + "TestEntity"."test_entity_pk" AS "TestEntity_test_entity_pk", + "TestEntity"."string_type" AS "TestEntity_string_type", + "TestEntity"."bool_type" AS "TestEntity_bool_type", + "TestEntity"."number_type" AS "TestEntity_number_type", + "TestEntity"."date_type" AS "TestEntity_date_type", + "TestEntity"."many_to_one_relation_id" AS "TestEntity_many_to_one_relation_id", + "TestEntity"."many_to_one_soft_delete_relation_id" AS "TestEntity_many_to_one_soft_delete_relation_id", + "TestEntity"."oneTestRelationTestRelationPk" AS "TestEntity_oneTestRelationTestRelationPk" +FROM + "test_entity" "TestEntity" + LEFT JOIN "test_relation" "testRelations" ON "testRelations"."test_entity_id" = "TestEntity"."test_entity_pk"" +`; + exports[`FilterQueryBuilder #select with relation should select and map relation 1`] = ` "SELECT "TestEntity"."test_entity_pk" AS "TestEntity_test_entity_pk", diff --git a/packages/query-typeorm/__tests__/query/filter-query.builder.spec.ts b/packages/query-typeorm/__tests__/query/filter-query.builder.spec.ts index f894271c3..da1c8a61f 100644 --- a/packages/query-typeorm/__tests__/query/filter-query.builder.spec.ts +++ b/packages/query-typeorm/__tests__/query/filter-query.builder.spec.ts @@ -314,45 +314,111 @@ describe('FilterQueryBuilder', (): void => { instance(mockWhereBuilder) ) }) - }) - it('should use skip/take when filtering on one to many relation', () => { - const mockWhereBuilder = mock>(WhereBuilder) - when(mockWhereBuilder.build(anything(), anything(), anything(), anything())).thenCall((qb: WhereExpressionBuilder) => qb) + it('should use skip/take when filtering on nested many to many relation', () => { + const mockWhereBuilder = mock>(WhereBuilder) + when(mockWhereBuilder.build(anything(), anything(), anything(), anything())).thenCall( + (qb: WhereExpressionBuilder) => qb + ) - expectSelectSQLSnapshot( - { - paging: { limit: 10, offset: 3 }, - filter: { testRelations: { testRelationPk: { eq: 'test' } } } - }, - instance(mockWhereBuilder) - ) - }) + expectSelectSQLSnapshot( + { + paging: { limit: 10, offset: 3 }, + filter: { oneTestRelation: { manyTestEntities: { testEntityPk: { eq: 'test' } } } } + }, + instance(mockWhereBuilder) + ) + }) - it('should use limit/offset when filtering on one to one relation', () => { - const mockWhereBuilder = mock>(WhereBuilder) - when(mockWhereBuilder.build(anything(), anything(), anything(), anything())).thenCall((qb: WhereExpressionBuilder) => qb) + it('should use skip/take when filtering on one to many relation', () => { + const mockWhereBuilder = mock>(WhereBuilder) + when(mockWhereBuilder.build(anything(), anything(), anything(), anything())).thenCall( + (qb: WhereExpressionBuilder) => qb + ) - expectSelectSQLSnapshot( - { - paging: { limit: 10, offset: 3 }, - filter: { oneTestRelation: { testRelationPk: { eq: 'test' } } } - }, - instance(mockWhereBuilder) - ) - }) + expectSelectSQLSnapshot( + { + paging: { limit: 10, offset: 3 }, + filter: { testRelations: { testRelationPk: { eq: 'test' } } } + }, + instance(mockWhereBuilder) + ) + }) - it('should use limit/offset when filtering on many to one relation', () => { - const mockWhereBuilder = mock>(WhereBuilder) - when(mockWhereBuilder.build(anything(), anything(), anything(), anything())).thenCall((qb: WhereExpressionBuilder) => qb) + it('should use skip/take when filtering on nested one to many relation', () => { + const mockWhereBuilder = mock>(WhereBuilder) + when(mockWhereBuilder.build(anything(), anything(), anything(), anything())).thenCall( + (qb: WhereExpressionBuilder) => qb + ) - expectSelectSQLSnapshot( - { - paging: { limit: 10, offset: 3 }, - filter: { manyToOneRelation: { testRelationPk: { eq: 'test' } } } - }, - instance(mockWhereBuilder) - ) + expectSelectSQLSnapshot( + { + paging: { limit: 10, offset: 3 }, + filter: { oneTestRelation: { testEntityRelation: { testRelationId: { eq: 'test' } } } } + }, + instance(mockWhereBuilder) + ) + }) + + it('should use limit/offset when filtering on one to one relation', () => { + const mockWhereBuilder = mock>(WhereBuilder) + when(mockWhereBuilder.build(anything(), anything(), anything(), anything())).thenCall( + (qb: WhereExpressionBuilder) => qb + ) + + expectSelectSQLSnapshot( + { + paging: { limit: 10, offset: 3 }, + filter: { oneTestRelation: { testRelationPk: { eq: 'test' } } } + }, + instance(mockWhereBuilder) + ) + }) + + it('should use limit/offset when filtering on nested one to one relation', () => { + const mockWhereBuilder = mock>(WhereBuilder) + when(mockWhereBuilder.build(anything(), anything(), anything(), anything())).thenCall( + (qb: WhereExpressionBuilder) => qb + ) + + expectSelectSQLSnapshot( + { + paging: { limit: 10, offset: 3 }, + filter: { oneTestRelation: { oneTestEntity: { testEntityPk: { eq: 'test' } } } } + }, + instance(mockWhereBuilder) + ) + }) + + it('should use limit/offset when filtering on many to one relation', () => { + const mockWhereBuilder = mock>(WhereBuilder) + when(mockWhereBuilder.build(anything(), anything(), anything(), anything())).thenCall( + (qb: WhereExpressionBuilder) => qb + ) + + expectSelectSQLSnapshot( + { + paging: { limit: 10, offset: 3 }, + filter: { manyToOneRelation: { testRelationPk: { eq: 'test' } } } + }, + instance(mockWhereBuilder) + ) + }) + + it('should use limit/offset when filtering on nested many to one relation', () => { + const mockWhereBuilder = mock>(WhereBuilder) + when(mockWhereBuilder.build(anything(), anything(), anything(), anything())).thenCall( + (qb: WhereExpressionBuilder) => qb + ) + + expectSelectSQLSnapshot( + { + paging: { limit: 10, offset: 3 }, + filter: { oneTestRelation: { testEntity: { testEntityPk: { eq: 'test' } } } } + }, + instance(mockWhereBuilder) + ) + }) }) }) diff --git a/packages/query-typeorm/src/query/filter-query.builder.ts b/packages/query-typeorm/src/query/filter-query.builder.ts index 4c7875c69..edae1aef8 100644 --- a/packages/query-typeorm/src/query/filter-query.builder.ts +++ b/packages/query-typeorm/src/query/filter-query.builder.ts @@ -18,6 +18,7 @@ import { UpdateQueryBuilder, WhereExpressionBuilder } from 'typeorm' +import { RelationMetadata } from 'typeorm/metadata/RelationMetadata' import { SoftDeleteQueryBuilder } from 'typeorm/query-builder/SoftDeleteQueryBuilder' import { AggregateBuilder } from './aggregate.builder' @@ -327,31 +328,25 @@ export class FilterQueryBuilder { } /** - * Checks if the query should use skip/take instead of limit/offset + * Checks if the query should use skip/take instead of limit/offset. + * + * We need to use Skip/Take instead of Limit/Offset when the query involves a join that might be (one|many)-to-many. + * This method looks for any n-to-many relations in the filter and if it finds any, it returns true. + * + * Recursively traverses the filter so we can detect nested n-to-many relations. */ - private shouldUseSkipTake(filter?: Filter): boolean { - if (!filter) { - return false - } + private shouldUseSkipTake(filter?: Filter, relations: RelationMetadata[] = this.repo.metadata.relations): boolean { + if (!filter) return false - return ( - getFilterFields(filter).filter((field) => { - const relation = this.repo.metadata.relations.find(({ propertyName }) => propertyName === field) + return getFilterFields(filter).some((field) => { + const relation = relations.find(({ propertyName }) => propertyName === field) + if (!relation) return false + if (!relation.isOneToOne && !relation.isManyToOne) return true - if (!relation) { - return false - } + const nestedFilter = filter[field] as Filter - if (!relation || relation.isOneToOne || relation.isManyToOne) { - return false - // } else if (relation.isOneToMany) { - // TODO - // return false - } else { - return true - } - }).length > 0 - ) + return this.shouldUseSkipTake(nestedFilter, relation.inverseEntityMetadata.relations) + }) } public getReferencedRelationsWithAliasRecursive(