Skip to content

fix: [branch-54] keep GROUP BY-less aggregates when propagating empty stages (backport of #2194) - #2235

Merged
andygrove merged 1 commit into
apache:branch-54from
andygrove:fix/grouping-less-aggregate-empty-branch-54
Aug 6, 2026
Merged

fix: [branch-54] keep GROUP BY-less aggregates when propagating empty stages (backport of #2194)#2235
andygrove merged 1 commit into
apache:branch-54from
andygrove:fix/grouping-less-aggregate-empty-branch-54

Conversation

@andygrove

Copy link
Copy Markdown
Member

Which issue does this PR close?

Backport of #2194 to branch-54. The issue it fixes is #2185.

Rationale for this change

With the adaptive planner, a query stage that completes with zero rows makes ExchangeExec::partition_statistics report Precision::Exact(0), and PropagateEmptyExecRule then collapses the plan above it on the next replan.

That rule replaced any AggregateExec whose input had become empty with an EmptyExec. This is only valid when the aggregate has a GROUP BY. An aggregate with no grouping expressions emits exactly one row even over zero input rows (sum returns NULL, count returns 0), so collapsing it silently drops that row. DataFusion's logical PropagateEmptyRelation rule carries the same guard (!agg.group_expr.is_empty()); the physical port here was missing it.

TPC-DS q61 hits this at SF1 because no store has s_gmt_offset = -7, so the store stage legitimately returns zero rows. The correct answer is a single all-NULL row, which single-process DataFusion and the static planner both produce, but the adaptive planner returned no rows at all.

This is a silent wrong answer rather than a failure, which is why it is worth carrying onto the release branch.

What changes are included in this PR?

A clean cherry-pick of 7eeff3b, unmodified.

Guards the AggregateExec arm of PropagateEmptyExecRule on !aggregation.group_expr().is_empty(), so a grouping-less aggregate is left in place over an empty input.

Adds three unit tests covering the aggregate arm: the grouped case still collapses, and the Single and Partial grouping-less cases are preserved. Both new grouping-less tests fail before the change.

Are there any user-facing changes?

No API changes. Queries run under ballista.planner.adaptive.enabled=true that contain a grouping-less aggregate over an input that turns out to be empty now return the correct single row instead of no rows. The static planner is unaffected.


Verified locally on the branch-54 base: cargo fmt --all -- --check is clean, and cargo check --workspace --all-targets --locked completes with no warnings on a combined stack of the six backports being proposed together. Test execution is left to CI.

@andygrove
andygrove marked this pull request as ready for review August 6, 2026 12:47
@andygrove
andygrove merged commit 7ceb0ff into apache:branch-54 Aug 6, 2026
17 checks passed
@andygrove
andygrove deleted the fix/grouping-less-aggregate-empty-branch-54 branch August 6, 2026 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants