Merge pull request #6006 from neo4j/fix-performance

Fix performance in new aggregations
neo4j · Feb 18, 2025 · d84f56c · d84f56c
2 parents d9d8300 + b105601
commit d84f56c
Show file tree

Hide file tree

Showing 8 changed files with 85 additions and 30 deletions.
diff --git a/packages/graphql/src/translate/queryAST/ast/operations/AggregationOperation.ts b/packages/graphql/src/translate/queryAST/ast/operations/AggregationOperation.ts
@@ -103,7 +103,8 @@ export class AggregationOperation extends Operation {
         const clauses = this.transpileAggregation(context);
 
         const isTopLevel = !(this.entity instanceof RelationshipAdapter);
-        if (isTopLevel || this.isInConnectionField) {
+        if (isTopLevel && !this.isInConnectionField) {
+            // This is to support deprecated aggregations
             const clausesSubqueries = clauses.flatMap((sq) => new Cypher.Call(sq));
 
             return {

diff --git a/packages/graphql/src/translate/queryAST/ast/operations/ConnectionReadOperation.ts b/packages/graphql/src/translate/queryAST/ast/operations/ConnectionReadOperation.ts
@@ -139,7 +139,10 @@ export class ConnectionReadOperation extends Operation {
     }
 
     public transpile(context: QueryASTContext): OperationTranspileResult {
-        if (!context.target) throw new Error();
+        if (!context.hasTarget())
+            throw new Error(
+                "Error generating query: contxt has no target in ConnectionReadOperation. This is likely a bug with the @neo4j/graphql library"
+            );
 
         // eslint-disable-next-line prefer-const
         let { selection: selectionClause, nestedContext } = this.selection.apply(context);
@@ -163,7 +166,18 @@ export class ConnectionReadOperation extends Operation {
 
         const filtersSubqueries = [...authFilterSubqueries, ...normalFilterSubqueries];
 
-        const aggregationSubqueries = this.aggregationField?.getSubqueries(context) ?? [];
+        // Only add the import if it is nested
+        const isTopLevel = !this.relationship;
+
+        const aggregationSubqueries = (this.aggregationField?.getSubqueries(context) ?? []).map((sq) => {
+            const subquery = new Cypher.Call(sq);
+            if (!isTopLevel) {
+                return subquery.importWith(context.target);
+            } else {
+                return subquery;
+            }
+        });
+
         const aggregationProjection = this.aggregationField?.getProjectionField() ?? {};
 
         const edgesVar = new Cypher.NamedVariable("edges");

diff --git a/packages/graphql/tests/performance/graphql/aggregations.graphql b/packages/graphql/tests/performance/graphql/aggregations.graphql
@@ -8,6 +8,39 @@ query TopLevelAggregate {
 }
 
 query TopLevelAggregateWithMultipleFields {
+    peopleConnection {
+        aggregate {
+            count {
+                nodes
+            }
+            node {
+                name {
+                    shortest
+                }
+                born {
+                    max
+                }
+            }
+        }
+    }
+}
+
+query NestedAggregation {
+    people {
+        name
+        moviesConnection {
+            aggregate {
+                node {
+                    title {
+                        longest
+                    }
+                }
+            }
+        }
+    }
+}
+
+query TopLevelAggregateWithMultipleFieldsDeprecated {
     peopleAggregate {
         count
         name {
@@ -19,7 +52,7 @@ query TopLevelAggregateWithMultipleFields {
     }
 }
 
-query NestedAggregation {
+query NestedAggregationDeprecated {
     people {
         name
         moviesAggregate {

diff --git a/packages/graphql/tests/performance/graphql/delete.graphql b/packages/graphql/tests/performance/graphql/delete.graphql
@@ -3,11 +3,3 @@ mutation SimpleDelete {
         nodesDeleted
     }
 }
-
-mutation NestedDeleteInUpdate {
-    updateMovies(delete: { actors: { where: { node: { name_CONTAINS: "Shark" } } } }) {
-        movies {
-            title
-        }
-    }
-}
diff --git a/packages/graphql/tests/performance/graphql/issues/2871.graphql b/packages/graphql/tests/performance/graphql/issues/2871.graphql
@@ -1,5 +1,5 @@
 query NestedRelationshipFilter {
-    movies(where: { actors: { movies_SOME: { title: "The Matrix" } } }) {
+    movies(where: { actors_SOME: { movies_SOME: { title: "The Matrix" } } }) {
         title
     }
 }
diff --git a/packages/graphql/tests/performance/graphql/issues/2925.graphql b/packages/graphql/tests/performance/graphql/issues/2925.graphql
@@ -5,7 +5,7 @@ query SingleRelationshipFilter {
 }
 
 query NestedSingleRelationshipFilter {
-    people(where: { movies: { favouriteActor: { name_IN: ["Tom Hanks"] } } }) {
+    people(where: { movies_SOME: { favouriteActor: { name_IN: ["Tom Hanks"] } } }) {
         name
     }
 }
@@ -17,7 +17,7 @@ query SingleRelationshipRequiredFilter {
 }
 
 query NestedSingleRelationshipRequiredFilter {
-    personClones(where: { movies: { favouriteActor: { name_IN: ["Tom Hanks"] } } }) {
+    personClones(where: { movies_SOME: { favouriteActor: { name_IN: ["Tom Hanks"] } } }) {
         name
     }
 }
diff --git a/packages/graphql/tests/performance/graphql/query.graphql b/packages/graphql/tests/performance/graphql/query.graphql
@@ -13,13 +13,6 @@ query SimpleQueryWithRelationship {
     }
 }
 
-# https://github.com/neo4j/graphql/issues/187
-query QueryWhere {
-    movies(where: { actors: { name: "Keanu Reeves" } }) {
-        released
-    }
-}
-
 query SimpleQueryWithNestedWhere {
     movies(where: { actors_SOME: { name: "Keanu Reeves" } }) {
         title
@@ -108,9 +101,9 @@ query OrFilterOnRelationshipsAndNested {
                 { actors_SOME: { born: 1997 } }
                 { actors_SOME: { born: 1998 } }
                 { actors_SOME: { born: 1956 } }
-                { directors: { movies: { title: "Matrix" } } }
-                { directors: { movies: { title: "foo" } } }
-                { directors: { movies: { title: "bar" } } }
+                { directors_SOME: { movies_SOME: { title: "Matrix" } } }
+                { directors_SOME: { movies_SOME: { title: "foo" } } }
+                { directors_SOME: { movies_SOME: { title: "bar" } } }
             ]
         }
     ) {

diff --git a/.../graphql/tests/tck/aggregations/field-level-aggregations/field-level-aggregations.test.ts b/.../graphql/tests/tck/aggregations/field-level-aggregations/field-level-aggregations.test.ts
@@ -49,8 +49,13 @@ describe("Field Level Aggregations", () => {
             query {
                 movies {
                     title
-                    actorsAggregate {
-                        count
+                    actorsConnection {
+                        aggregate {
+                            count {
+                                nodes
+                                edges
+                            }
+                        }
                     }
                 }
             }
@@ -62,10 +67,27 @@ describe("Field Level Aggregations", () => {
             "MATCH (this:Movie)
             CALL {
                 WITH this
-                MATCH (this)<-[this0:ACTED_IN]-(this1:Actor)
-                RETURN count(this1) AS var2
+                CALL {
+                    WITH this
+                    MATCH (this)<-[this0:ACTED_IN]-(this1:Actor)
+                    RETURN { nodes: count(this1), edges: count(this0) } AS var2
+                }
+                CALL {
+                    WITH *
+                    MATCH (this)<-[this3:ACTED_IN]-(this4:Actor)
+                    WITH collect({ node: this4, relationship: this3 }) AS edges
+                    WITH edges, size(edges) AS totalCount
+                    CALL {
+                        WITH edges
+                        UNWIND edges AS edge
+                        WITH edge.node AS this4, edge.relationship AS this3
+                        RETURN collect({ node: { __id: id(this4), __resolveType: \\"Actor\\" } }) AS var5
+                    }
+                    RETURN var5, totalCount
+                }
+                RETURN { edges: var5, totalCount: totalCount, aggregate: { count: var2 } } AS var6
             }
-            RETURN this { .title, actorsAggregate: { count: var2 } } AS this"
+            RETURN this { .title, actorsConnection: var6 } AS this"
         `);
 
         expect(formatParams(result.params)).toMatchInlineSnapshot(`"{}"`);