perf(query-typeorm): Don't use skip/take when filtering for one to on…

…e and many to one relations
TriPSs · Feb 19, 2023 · cdd06b6 · cdd06b6
1 parent 1f5ab0d
commit cdd06b6
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 18 deletions.
diff --git a/packages/query-typeorm/src/query/filter-query.builder.ts b/packages/query-typeorm/src/query/filter-query.builder.ts
@@ -78,7 +78,6 @@ export class FilterQueryBuilder<Entity> {
    * @param query - the query to apply.
    */
   public select(query: Query<Entity>): SelectQueryBuilder<Entity> {
-    const hasFilterRelations = this.filterHasRelations(query.filter)
     let qb = this.createQueryBuilder()
     qb = this.applyRelationJoinsRecursive(
       qb,
@@ -87,24 +86,12 @@ export class FilterQueryBuilder<Entity> {
     )
     qb = this.applyFilter(qb, query.filter, qb.alias)
     qb = this.applySorting(qb, query.sorting, qb.alias)
-    qb = this.applyPaging(qb, query.paging, hasFilterRelations)
+    qb = this.applyPaging(qb, query.paging, this.shouldUseSkipTake(query.filter))
     return qb
   }
 
   public selectById(id: string | number | (string | number)[], query: Query<Entity>): SelectQueryBuilder<Entity> {
-    const hasFilterRelations = this.filterHasRelations(query.filter)
-
-    let qb = this.createQueryBuilder()
-    qb = this.applyRelationJoinsRecursive(
-      qb,
-      this.getReferencedRelationsRecursive(this.repo.metadata, query.filter, query.relations),
-      query.relations
-    )
-    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, hasFilterRelations)
-    return qb
+    return this.select(query).andWhereInIds(id)
   }
 
   public aggregate(query: Query<Entity>, aggregate: AggregateQuery<Entity>): SelectQueryBuilder<Entity> {
@@ -300,6 +287,34 @@ export class FilterQueryBuilder<Entity> {
     return getFilterFields(filter).filter((f) => relationNames.includes(f)).length > 0
   }
 
+  /**
+   * Checks if the query should use skip/take instead of limit/offset
+   */
+  private shouldUseSkipTake(filter?: Filter<Entity>): boolean {
+    if (!filter) {
+      return false
+    }
+
+    return (
+      getFilterFields(filter).filter((field) => {
+        const relation = this.repo.metadata.relations.find(({ propertyName }) => propertyName === field)
+
+        if (!relation) {
+          return false
+        }
+
+        if (!relation || relation.isOneToOne || relation.isManyToOne) {
+          return false
+          // } else if (relation.isOneToMany) {
+          //   TODO
+          // return false
+        } else {
+          return true
+        }
+      }).length > 0
+    )
+  }
+
   public getReferencedRelationsRecursive(
     metadata: EntityMetadata,
     filter: Filter<unknown> = {},

diff --git a/packages/query-typeorm/src/services/relation-query.service.ts b/packages/query-typeorm/src/services/relation-query.service.ts
@@ -254,7 +254,7 @@ export abstract class RelationQueryService<Entity> {
    * @param relationId - The id of the relation to set on the entity.
    * @param opts - Additional options
    */
-  async setRelation<Relation>(
+  public async setRelation<Relation>(
     relationName: string,
     id: string | number,
     relationId: string | number,
@@ -276,7 +276,7 @@ export abstract class RelationQueryService<Entity> {
    * @param relationIds - The ids of the relations to add.
    * @param opts - Additional options
    */
-  async removeRelations<Relation>(
+  public async removeRelations<Relation>(
     relationName: string,
     id: string | number,
     relationIds: (string | number)[],
@@ -298,7 +298,7 @@ export abstract class RelationQueryService<Entity> {
    * @param relationName - The name of the relation to query for.
    * @param relationId - The id of the relation to set on the entity.
    */
-  async removeRelation<Relation>(
+  public async removeRelation<Relation>(
     relationName: string,
     id: string | number,
     relationId: string | number,