andygrove opened a new pull request, #2200: URL: https://github.com/apache/datafusion-ballista/pull/2200
# Which issue does this PR close? N/A — follow-up to the review of #2194 (https://github.com/apache/datafusion-ballista/pull/2194#pullrequestreview-4797353244), which fixed #2185 for GROUP BY-less aggregates and flagged that grouping sets have the same problem. While fixing that, an audit of the other `PropagateEmptyExecRule` arms found three more cases where the rewrite changes query semantics or plan shape. # Rationale for this change `PropagateEmptyExecRule` is the physical port of DataFusion's logical `PropagateEmptyRelation` rule, and physical plans carry details that logical plans do not (embedded projections, output partitioning, grouping-set null masks). Four arms of the rule did not account for them: 1. **Empty grouping sets.** The logical rule guards its aggregate arm on both `!agg.group_expr.is_empty()` and `!has_empty_grouping_set(&agg.group_expr)`; #2194 ported only the first. `GROUPING SETS ((a), ())`, `ROLLUP(a)`, and `CUBE(a)` all contain the empty grouping set `()`, which emits one grand-total row even over zero input rows (DataFusion 54 implements this in `init_empty_grouping_sets` in `row_hash.rs`). Collapsing such an aggregate to `EmptyExec` silently drops that row — the same wrong-answer class as #2185. 2. **Hash joins with embedded projections.** The join rewrites that keep one input alive assume the join schema is `left fields ++ right fields`: the null-padded projection for Left/Right/Full maps columns positionally, and the anti-join arms return the raw surviving child. `ProjectionPushdown` can embed a projection into `HashJoinExec`, making the join schema a subset/reorder of the inputs; the null-padding then silently reads the wrong columns, and the anti-join replacement changes the plan's schema. The arms that collapse to `EmptyExec` use the join's own projected schema and were already correct. 3. **Filters with embedded projections.** The `FilterExec` arm returned the filter's input; with an embedded projection the filter's schema differs from its input's, so the rewrite changed the schema. 4. **Exchange over an empty input.** The `ExchangeExec` arm returned the input `EmptyExec` directly, discarding the exchange's output partition count that parent operators were planned against. The stats-based exchange arm a few lines below already preserves the partition count; the two arms now match. # What changes are included in this PR? - Guard the aggregate arm on the physical equivalent of `has_empty_grouping_set`: any group whose null mask is all-`true` is the empty grouping set, and the aggregate is preserved. - Add `has_projection` to `JoinInfo` (`HashJoinExec::contains_projection()`; `SortMergeJoinExec` has no embedded projection) and skip the surviving-input rewrites when it is set, leaving the join untouched. - Replace `FilterExec` and `ExchangeExec` over empty inputs with an `EmptyExec` built from the operator's own schema and partition count instead of returning the input. Tests: the grouping-sets test contributed by @avantgardnerio in the #2194 review (cherry-picked, keeping authorship), plus four new tests covering Left/LeftAnti joins with embedded projections, a filter with an embedded projection, and exchange partition-count preservation. All five fail before the fix. # Are there any user-facing changes? No API changes (`JoinInfo` gains a public field). Queries run under `ballista.planner.adaptive.enabled=true` where a stage turns out empty now return correct results for grouping-set aggregates and for joins/filters with embedded projections, and no longer lose the exchange's partition count when a stage collapses. -- 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]
