perf: Remove an aggregate whose GROUP BY covers a unique key - #24789
Draft
Dandandan wants to merge 2 commits into
Draft
perf: Remove an aggregate whose GROUP BY covers a unique key#24789Dandandan wants to merge 2 commits into
Dandandan wants to merge 2 commits into
Conversation
`Unnest::try_new` copied the input's functional dependencies unchanged, with
the comment "We can use the existing functional dependencies". That is not
true for a list unnest, which turns one input row into several. A determinant
that occurred once in the input can occur many times in the output.
Optimizer rules that read those dependencies then produce wrong results. With
a declared key:
CREATE TABLE t_list (k INT, vals INT[], PRIMARY KEY (k))
AS VALUES (1, [10, 20, 30]), (2, [40]);
CREATE TABLE t_join (k INT) AS VALUES (1), (2);
SELECT u.k, u.v FROM (SELECT k, unnest(vals) AS v FROM t_list) u
JOIN t_join j ON u.k = j.k;
returns 2 rows instead of 4, because `eliminate_join` sees `u` as unique on
`k` and rewrites the inner join into a semi join, which drops the repeated
rows. Without the PRIMARY KEY the same query returns 4.
The dependency still holds in the weaker sense: all rows produced from one
input row share the determinant, so it determines the same columns. Downgrade
it to `Dependency::Multi` rather than dropping it, which keeps it useful and
stops it being read as a uniqueness guarantee.
Unnesting a struct produces one row per input row, so those dependencies are
left as they are.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgqwvctZdvR1ZCz2hbkEJC
When the GROUP BY expressions include a unique key of the input, every group
holds exactly one row. The aggregate then produces one output row per input
row, like a projection, and each aggregate function returns the value of that
single row.
DataFusion did not use this. With lineitem keyed by (l_orderkey, l_linenumber):
SELECT l_orderkey, l_linenumber, sum(l_quantity)
FROM lineitem GROUP BY l_orderkey, l_linenumber
still built a hash table over all 60M rows, one row per group.
This extends `EliminateGroupByConstant`, which already simplifies an aggregate
based on what its GROUP BY contains and runs at the right point, so it costs no
extra plan traversal. The aggregate becomes a projection:
Projection: l_orderkey, l_linenumber, CAST(l_quantity AS Decimal128(25, 2))
TableScan: lineitem
On TPC-H SF10 with the key declared, summing that result goes from 751 ms to
31 ms. A DISTINCT over a unique key, which the planner turns into an aggregate
with no aggregate expressions, goes from 51 ms to 19 ms.
The single-row value of each aggregate is known for min, max, sum, avg,
first_value and last_value (the argument, cast to the aggregate's return type
where that differs) and count (1, or 0 when the argument is NULL). Anything
else keeps the aggregate. DISTINCT, ORDER BY and IGNORE NULLS make no
difference to one row, but a FILTER can exclude it and leave the aggregate with
no input at all, so those are left alone.
An empty GROUP BY is not eligible: it returns one row for an empty input, which
a projection would not. Grouping sets are not eligible either, since they do
not group by every expression at once.
Of the benchmark suites, only TPC-DS q54 changes: it drops a DISTINCT over
customer's primary key. Its runtime is unchanged at SF1.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgqwvctZdvR1ZCz2hbkEJC
Dandandan
force-pushed
the
feat/eliminate-aggregate-superkey
branch
from
August 30, 2026 07:40
3943ad7 to
9988cb9
Compare
2010YOUY01
self-requested a review
August 30, 2026 12:24
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.
Which issue does this PR close?
Rationale for this change
When the GROUP BY expressions include a unique key of the input, every group
holds exactly one row. The aggregate then produces one output row per input
row, like a projection, and every aggregate function returns the value of that
single row.
DataFusion did not use this. With
lineitemkeyed by(l_orderkey, l_linenumber):still built a hash table over all 60M rows to put one row in each group.
What changes are included in this PR?
EliminateGroupByConstantalready simplifies an aggregate based on what itsGROUP BY contains, matches on
Aggregate, and runs at the right point, so thecheck goes there rather than in a new rule and costs no extra plan traversal.
Its name is now a little narrow for what it does; happy to rename it if
reviewers prefer.
The aggregate becomes a projection:
The single-row value is known for
min,max,sum,avg,first_valueandlast_value(the argument, cast to the aggregate's return type where thatdiffers) and for
count(1, or 0 when the argument is NULL). Any otheraggregate keeps the aggregate node.
DISTINCT,ORDER BYandIGNORE NULLSmake no difference to a group of onerow. A
FILTERdoes: it can exclude the row and leave the aggregate with noinput at all, which is a different value for every function, so those keep the
aggregate.
Two cases are not eligible:
GROUP BY, which returns one row for an empty input where aprojection returns none;
them proves nothing.
The rule also removes a
SELECT DISTINCTover a unique key, which the plannerturns into an aggregate with no aggregate expressions.
Benchmarks
TPC-H SF10 with the keys declared, median of 5, three interleaved rounds:
sum(q)overGROUP BY l_orderkey, l_linenumberSELECT DISTINCT c_custkey, c_name, c_addressBoth return identical results.
Of the benchmark suites, only TPC-DS q54 changes.
dfbenchdeclares primarykeys for both TPC-H and TPC-DS, and with those declared q54 drops a
DISTINCTover
customer's primary key. Its runtime is unchanged at SF1 (50.4 ms beforeand after, median of 7 over three rounds), since the aggregate removed is over
a dimension table. Every other TPC-H, TPC-DS and ClickBench plan is byte for
byte identical, checked by diffing all 184 optimized plans against
main.Are these changes tested?
Yes.
functional_dependencies.sltgains a section covering the rewrite foreach supported aggregate, NULL handling for all of them, extra grouping
expressions beyond the key, and the four cases that keep the aggregate (FILTER,
grouping sets, an unsupported aggregate, and grouping by a non-key).
The rule fires in existing tests in
aggregate.slt,distinct_on.slt,explain.slt,functional_dependencies.sltandgroup_by.slt; those expectedplans are updated and no expected result changes. The full sqllogictest suite
(504 files) passes.
Are there any user-facing changes?
Queries that group by a unique key, or take a DISTINCT over one, no longer run
an aggregate. Results are unchanged.