andygrove opened a new pull request, #2132:
URL: https://github.com/apache/datafusion-ballista/pull/2132
# Which issue does this PR close?
Backport of #2062 to `branch-54`. The issue it fixes is #2047.
# Rationale for this change
With adaptive execution enabled (`ballista.planner.adaptive.enabled=true`),
15 TPC-DS queries fail on `54.x` with:
```
Internal("Assertion failed: partition < self.partitions:
EmptyExec invalid partition 1 (expected less than 1)")
```
The adaptive planner's `PropagateEmptyExecRule` collapses a provably empty
sub-plan into an `EmptyExec` that inherits the partition count of the node it
replaced, and the scheduler sizes a stage's task count from that count.
`datafusion-proto` encodes an `EmptyExec` as its schema alone —
`EmptyExecNode` has no partitions field, and the decoder rebuilds the node with
`EmptyExec::new(schema)`, which defaults to one partition. So a stage built
from an `EmptyExec` with N partitions launches N tasks against a plan that,
once it reaches the executor, can only execute partition 0. Every task above
partition 0 then trips the assertion.
# What changes are included in this PR?
A clean cherry-pick of 0f6ec8c6, unmodified.
Multi-partition `EmptyExec` nodes are rewritten into a round-robin
`RepartitionExec` over a single-partition `EmptyExec` when a stage plan is
built for the wire. The plan is equivalent, and a `RepartitionExec` carries its
partitioning through serialization. The rewrite runs after all physical
optimizer rules, so no later rule can collapse it.
Tests added by the original PR, all passing on the `branch-54` base:
- `serde::test::empty_exec_partition_count_is_lost_by_datafusion_proto` —
pins the underlying `datafusion-proto` behavior this works around
- `planner::test::multi_partition_empty_exec_is_rewritten_for_the_wire`
- `planner::test::single_partition_empty_exec_is_left_alone`
# Are there any user-facing changes?
Queries that previously failed with the `EmptyExec invalid partition`
assertion under AQE now run. No public API, config, or proto/wire format
changes, so this is not a breaking change.
The rewrite is a no-op for the static planner, which never produces a
multi-partition `EmptyExec`. I verified that explicitly: the TPC-H
plan-stability suite passes with the goldens **untouched**, so static plan
shapes are unaffected and only the adaptive path changes.
`cargo fmt --check`, clippy on `ballista-core`/`ballista-scheduler` with
`--all-targets --all-features`, and the full `ballista-core` +
`ballista-scheduler` test suites are clean.
--
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]
