EQL: portable aggregation over subqueries

[homedirectory]

Description

EQL queries that yield an aggregation over a non-trivial argument are not portable across databases.

SQL Server rejects an aggregate function whose argument contains a subquery, e.g. SUM((SELECT ...)), with:

Cannot perform an aggregate function on an expression containing an aggregate or a subquery.

PostgreSQL accepts the same construct. As a result, an EQL query such as select(X).groupBy(g).yield(sum(<expr>)) — where <expr> is a subquery (either literal or expanded from a calculated property) — succeeds with PostgreSQL but fails with SQL Server.

The goal of this issue is to make such queries portable by introducing a transformation that rewrites the query into an outer query which aggregates over a source query that first materialises the aggregate's argument as a plain column:

-- Before
select(S).groupBy().prop(g).yield().sum().<expr>.as("sum")

-- After
select( select(S).yield().prop(g).as("c1").yield().<expr>.as("c2") )
   .groupBy().prop("c1")
   .yield().sum().prop("c2").as("sum")

Scope. Only aggregations over non-trivial arguments are transformed — expressions, calculated properties, and sub-queries. Aggregations over a plain persistent property (e.g. sum(persistentProp)) are deliberately left untouched, because SQL Server handles those natively and wrapping them would add cost for no benefit.

Nested aggregation

Both PostgreSQL and SQL Server reject nested aggregation.

select sum(max(prop))

EQL rejects nested aggregations by design and it is NOT a goal of this issue to support such expressions.

1. The EQL grammar does not permit an aggregation as an argument of an aggregate function.

   For example, the query below is invalid: maxOf is not defined at the point of its invocation.

   yield().sumOf().maxOf()

   However, the following is possible:

   yield().sumOf().expr(expr().maxOf() ... .model())

2. Although it is possible to construct a nested aggregation through calculated properties, this does not make sense in practice.

   class Entity {
     @IsProperty
     @Calculated
     Integer totalCost;
     static final ExpressionModel totalCost_ = expr().maxOf().prop("cost").model();
   
     @IsProperty
     @Calculated
     Integer count;
     static final ExpressionModel count_ = expr().countAll().model();
   }

   Calculated properties that perform an aggregation at the level of their enclosing entity are used in practice only in entity types generated for Entity Centre totals, where they are always yielded directly, never within another aggregation.

   Therefore, queries that yield nested aggregations, as shown below, should never occur in practice.

   select(Entity.class).yield().sumOf().prop("totalCost")
   =>
   select(Entity.class).yield().sumOf().expr(expr().maxOf().prop("cost").model())
   

Change overview

Changes in this issue introduce a new transformation into the EQL compiler -- AggregateOperandMaterialiser.
This transformation applies to any query that yields or orders by an aggregation.
It materialises the arguments of aggregate functions as columns in a source query, which overcomes the SQL Server limitation of not being able to aggregate over subqueries directly.

An immediate practical benefit is the ability to use Entity Centre totals (addProp(...).withSummary(...)) on calculated properties whose expression contains a sub-query.

class ProjectCostDetails extends AbstractEntity<Project> {
  @Calculated
  @IsProperty
  BigDecimal totalCost;
  static final ExpressionModel totalCost_ = expr().model(
        select(WorkActivity.class)
          .where().prop("project").eq().extProp("key")
          .yield().sumOf().prop("cost")
          .modelAsPrimitive())
      .model()
}

class Project extends ... {
  // One-2-one implicitly calculated
  @IsProperty
  ProjectCostDetails costDetails;
}

class ProjectWebUiConfig {

  EntityCentre<Project> createCentre() {

      centreFor(Project.class)
        ...
        .addProp("costDetails.totalCost") ...
          .withSummary("sum_totalCost_", "SUM(costDetails.totalCost)", ...)

  }
  
}

The transformation is a no-op in the following cases:

• The query neither yields nor orders by an aggregation.
  In particular, group-by operands alone do not make a query eligible, regardless of their complexity.
• Every aggregate argument is a plain persistent property (e.g., sum(persistentProp)) -- SQL Server handles these natively, and wrapping them would add cost for no benefit.
• A yield or an order-by contains a subquery that would not be materialised (see Limitations).

The transformation applies to the stage 3 EQL AST, mainly because it contains calculated properties in their expanded form, simplifying analysis.

The transformation has been made to apply only to SQL Server.
This makes the SQL generated for other DBs more clear, allowing their respective optimisers to make their own best decisions.

As part of this issue, the WithDbVersion annotation was extended to apply to entire test classes, in addition to individual test methods.
A class-level annotation is inherited by subclasses and applies to inherited test methods; a method-level annotation takes precedence over a class-level one.
This supports test suites that are meaningful only for a specific database, such as the operational tests for this transformation.

Breaking changes

Some existing EQL queries are affected by changes in this issue.

One observed pattern that will result in EQL runtime errors is the following:

.groupBy().prop(WorkOrder.asset())
.yield().prop(WorkOrder.asset()).as(WorkOrder.asset())
.yield().prop(WorkOrder.asset().id()).as(ID) // INVALID

Assuming this query yields or orders by an aggregation, which will make it subject to the implemented transformation, the ID yield will be deemed invalid by the EQL compiler.

In short, yield asset is valid because it equals the group-by expression, and ID is invalid because it is neither equal to the group-by expression, nor is it an aggregation.

To correct such a query:

.groupBy().prop(WorkOrder.asset())
.yield().prop(WorkOrder.asset()).as(WorkOrder.asset())
- .yield().prop(WorkOrder.asset().id()).as(ID) // INVALID
+ .yield().prop(WorkOrder.asset()).as(ID)

Limitations

Subqueries outside aggregate arguments

The transformation's replacement operation does not descend into subqueries.
Therefore, a subquery that occurs in a yield or an order-by outside of an aggregate function's argument cannot be rewritten to account for the new query structure: any reference to the original source within it would become dangling after the transformation, as the original source moves into the source query.
When such a subquery is detected and it is not materialised itself, the transformation is skipped, and the query remains as is.

SQL Server can evaluate such an untransformed query unless it genuinely requires materialisation (an aggregate function's argument or a group-by operand contains a subquery), in which case SQL Server rejects it natively -- the same outcome as in the absence of this transformation.

The notable instance of this limitation is a subquery that is both grouped by and yielded:

countFuelUsage = select(FuelUsage.class).where().prop("vehicle").eq().extProp(ID).yield().countAll().modelAsPrimitive();
select(Vehicle.class)
    .groupBy().model(countFuelUsage)
    .yield().model(countFuelUsage).as("count") // Remove this yield and the transformation applies.
    .modelAsAggregate();

The group-by subquery would be materialised, but the yielded subquery could not reuse that column: the two subqueries are not seen as equal, hence cannot be materialised under one column.
The current implementation compares queries by [AbstractQuery3#equals], which considers generated identifiers (they differ between the two instances, so the queries are never equal).
Such queries are therefore skipped and fail on SQL Server, which does not support subqueries in GROUP BY.
Lifting this limitation is a subject of #2788.

Expected outcome

• EQL queries that aggregate over sub-queries, expressions, or calculated properties -- in yields or in order-bys -- produce equivalent results on SQL Server and PostgreSQL, removing a class of SQL-Server-only failures and improving database portability.
• Simple aggregations over persistent properties and non-aggregating queries are unaffected (no added query nesting or cost).
• Queries that the transformation cannot rewrite (see Limitations) are left untouched and behave on SQL Server exactly as they did before this change.

Future work

EQL AST: Rethinking equals and hashCode

Note: This work is tracked in #2788.

It would be great to rethink equals and hashCode for EQL AST nodes.
Currently, both methods traverse the whole tree, which can get quite expensive in large trees.

This cost was observed while implementing this issue: applying the transformation to a deeply nested query with many yields caused a significant drop in performance.
The transformation stores AST nodes in hash-based collections, and every Prop3.hashCode invocation traversed the whole tree of its source (a deeply nested Source3BasedOnQueries).
A hashCode cache for ISource3 nodes was introduced first (tg/commit/c2388d8929), but was subsequently reverted in favour of comparing sources by id in Prop3.equals and Prop3.hashCode (tg/commit/4df591acf2).
The new implementation is consistent with Prop2 and is sufficient: two property references are equal iff they refer to the same property of the same source, and sources are uniquely identified by their ids.
Notably, this is exactly the equality required by the transformation: groupBy().prop(x) and yield().prop(x) (two distinct Prop3 instances) are recognised as the same expression and materialised as a single column -- important for correctness, not just performance.

The general problem remains: equals and hashCode of composite nodes (e.g., Source3BasedOnQueries, AbstractQuery3) still traverse whole trees.

There appear to be three distinct notions of equality for EQL AST nodes, and a single Object.equals cannot serve all of them:

1. Identity for runtime bookkeeping -- cheap, id-based (perhaps even reference-based) equality, used by transformations to store and look up nodes in hash-based collections.
   Prop3 now follows this notion with respect to its source.
   Extending it to ISource3 itself and to query nodes is the open performance work.
   The transformation already resorts to identity semantics internally: the node replacement operation uses an IdentityHashMap because distinct occurrences may be structurally equal.

2. Structural equality, including generated ids -- what Object.equals implements today and what structural EQL tests rely on.
   If notion 1 takes over Object.equals, structural comparison should move into an explicit tree comparator used by tests.
   Otherwise, an expected node could be created with a matching id but a different structure, and would be incorrectly considered equal (false positive).
   A dedicated comparator could also produce structural diffs, improving assertion messages for large trees (cf. assertQueryEquals).

3. Structural equality modulo generated ids (alpha-equivalence) -- recognising two independently built, structurally identical trees as the same.
   This is what the transformation needs to unify subqueries: the limitation "Subqueries outside aggregate arguments" exists precisely because the two occurrences of the subquery are structurally identical yet never equal under notion 2, as generated ids are unique.
   With this notion available, both occurrences would be materialised under one column, lifting the limitation.

Although listed last, item 3 offers the most concrete benefit: it would lift the limitation described above, whereas items 1 and 2 are internal improvements (performance and test infrastructure, respectively).
A practical sequencing is roughly the reverse of the numbering: implement an explicit tree comparator with configurable sensitivity to generated ids (covering items 2 and 3), migrate the structural tests and the transformation's deduplication onto it, and only then simplify Object.equals and hashCode to cheap id-based identity (item 1).

Note that item 3 is more than id-blind comparison: ids bound within the compared trees are renameable, but free references (e.g. correlated references to enclosing sources) must be respected -- otherwise, two structurally identical subqueries correlated to different sources would be wrongly unified.