Fix performance in new aggregations #6006
Conversation
|
angrykoala
force-pushed
the
fix-performance
branch
from
da156e7 to
1c9bbf3
Compare
angrykoala
force-pushed
the
fix-performance
branch
from
1c9bbf3 to
9600435
Compare
| @@ -139,7 +139,8 @@ export class ConnectionReadOperation extends Operation { | |||
| } | |||
|
|
|||
| public transpile(context: QueryASTContext): OperationTranspileResult { | |||
| if (!context.target) throw new Error(); | |||
| const contextTarget = context.target; | |||
| if (!contextTarget) throw new Error(); | |||
There was a problem hiding this comment.
| if (!contextTarget) throw new Error(); | |
| if (!contextTarget.hasTarget()) throw new Error(); |
| @@ -51,7 +51,7 @@ export class CountField extends AggregationField { | |||
| } | |||
|
|
|||
| public getAggregationExpr(variable: Cypher.Variable): Cypher.Expr { | |||
| return Cypher.count(variable); | |||
| return Cypher.count(variable).distinct(); | |||
There was a problem hiding this comment.
Can you add this in a separate PR?
| return Cypher.count(variable).distinct(); | |
| return Cypher.count(variable).distinct(); |
| @@ -163,7 +164,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; | |||
There was a problem hiding this comment.
Nitpick: considering the use case of this variable instead assign a variable from a negation and then negate the same variable again I will suggest to convert the variable name to isNested and remove all the negations.
| @@ -7,7 +7,40 @@ query TopLevelAggregate { | |||
| } | |||
| } | |||
|
|
|||
| query TopLevelAggregateWithMultipleFields { | |||
| query TopLevelAggregateWithMultipleFields_only { | |||
| } | ||
| } | ||
|
|
||
| query TopLevelAggregateWithMultipleFieldsDeprecated_only { |
Description
This PR fixes the performance (and potential bug) on new aggregations due to a missing
WITH this