andygrove opened a new pull request, #2235: URL: https://github.com/apache/datafusion-ballista/pull/2235
# 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 7eeff3be, 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. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
