feat(sql): expand GROUPING SETS / ROLLUP / CUBE into merged per-level aggregates (#118) - #124
Merged
Merged
Conversation
… aggregates (#118) Multi-level grouping produces several grouping levels from one scan, while `Aggregate.by` holds a single key set. Expand each level into its own `Aggregate` and `Merge` them — option 1 from the issue, which needs no IR change. A level that omits a key must still emit it, as NULL, per SQL. So each level's Aggregate is wrapped in a Project that reinstates the omitted keys and restores the canonical column order. Without that the levels would not be union-compatible, and `Merge` derives its schema from the first child — every other branch would be silently misdescribed. The nulls are *cast* to the key's declared type: a bare Null literal infers as Float64. DataFusion's `__grouping_id` discriminator is dropped. It exists only to tell a subtotal's NULL from a data NULL, which is observable solely through `GROUPING(col)` — an aggregate this front end rejects as unsupported. A test pins that rejection, since it is what makes dropping the column sound. DataFusion normalizes every mixed form (`GROUP BY g, ROLLUP(d)`) into a single `GroupingSets`, so one grouping expression is the only shape to handle. Composes with #110: a derived reducer argument (`SUM(bytes * 8)`) materializes in a Project beneath every level's Aggregate. A non-column key *inside* a grouping set is rejected — it would also have to be reinstatable as a typed null — with a message that says so. The scan is duplicated per level; `plan::cse` hoists it back to one producer, the same trade `histogram_quantiles` makes (#109). CUBE(n) is 2^n levels by definition. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #118. Takes option 1 from the issue — expand to a
Mergeof oneAggregateper grouping level — which needs no IR change and reuses theMergepath #109 opened.Shape
Three things that were not obvious
A level that omits a key must still emit it, as
NULL. So each level'sAggregateis wrapped in aProjectthat reinstates the omitted keys and restores the canonical column order. Without it the levels are not union-compatible — andMergederives its schema from the first child, so every other branch would be silently misdescribed. Same trap as thehistogram_quantilesbranches in #121.The nulls have to be cast.
infer_expr_typemaps a bareL3Scalar::NulltoFloat64, so the()level ofROLLUP(service)would have emittedservice: Float64. They are cast to the key's declared type instead, and a test assertsUtf8.__grouping_idis safe to drop. DataFusion always adds it to the aggregate schema, but the enclosing Projection never references it. Its only purpose is to distinguish a subtotal'sNULLfrom a dataNULL, which is observable solely throughGROUPING(col)— andGROUPINGis an aggregate this front end rejects (unsupported aggregate: grouping).grouping_function_is_rejectedpins that, since it is precisely what makes dropping the column sound.Also verified
GROUP BY g, ROLLUP(d)) into a singleGroupingSets, sogroup_expris always exactly one grouping expression. One shape to handle, not three.distinct_expr()is ordered like the aggregate's leading schema fields, which is the column order the enclosing Projection expects.SUM(bytes * 8)materializes in a derived-columnProjectbeneath every level's Aggregate.WHEREstill folds onto the Scan;HAVING/ORDER BY/LIMITsit above theMergeand work.ROLLUP(date_trunc(...))) is rejected — it would also need to be reinstatable as a typed null — with a message that says so rather than the generic SQL: expression GROUP BY (date_trunc time bucketing), aggregates over expressions, and GROUPING SETS/ROLLUP/CUBE are rejected #110 one.Cost
The scan is duplicated per level;
plan::csehoists it back into a single producer, the same tradehistogram_quantilesmakes.CUBE(n)is2^nlevels by definition, so the IR tree grows with it — no artificial cap added, since DataFusion builds the same power set.Tests (8 new, 1 rewritten)
multi_level_grouping_is_rejected(added by #110) is replaced bymulti_level_grouping_lowers_to_one_aggregate_per_level. The rest pin prefix expansion, the power set, mixed-form normalization, typed nulls, union compatibility, theGROUPING()rejection, the expression-key rejection, and composition with #110.Verification
🤖 Generated with Claude Code