feat: execute uncorrelated scalar subqueries first instead of rewriting to a join#1912
feat: execute uncorrelated scalar subqueries first instead of rewriting to a join#1912andygrove wants to merge 10 commits into
Conversation
Bump the workspace to the published datafusion 54 (and arrow/arrow-flight 58.3, object_store 0.13.2, rustyline 18 in ballista-cli), replacing the branch-54 git-rev pin from the earlier draft. API migration to 54: - as_any removed from ExecutionPlan/DataSource/PhysicalExpr: downcast directly on the trait object (and upcast dyn ShuffleWriter to dyn ExecutionPlan); as_any retained where it still exists (Arrow arrays, UserDefinedLogicalNode, ExtensionOptions). - ExecutionPlan::partition_statistics returns Result<Arc<Statistics>>. - parse_protobuf_partitioning/_hash_partitioning take a PhysicalPlanDecodeContext; serialize_partitioning takes the codec plus the proto converter. - TaskContext::new and FunctionRegistry gain higher-order-function arguments (wired empty in Ballista). - BatchPartitioner::new_hash_partitioner is fallible. Test updates for 54 behavior changes: - approx_percentile_cont_with_weight result; shuffle-writer tests made robust to hash distribution; execution-graph/dot tests use Ballista session config so the small join stays Partitioned; regenerated AQE plan snapshots (projection folded into HashJoinExec).
DataFusion 54's DataSourceExec hands file groups to partition streams from a shared work-queue that is only divided across partitions when all partitions of one plan instance are polled concurrently. Ballista executes one partition per task on its own decoded plan instance, so a task that polls a single partition in isolation drains the whole queue and scans the entire table -- producing N-times-inflated results and N-times the work for every task dispatched after the first wave. Restrict each task's DataSourceExec to the file group for its partition_id before execution (other group slots emptied, partition count preserved) so the lone execute(partition_id) reads only its own group. Skip when partition_id is outside the source's file groups (an operator between the scan and the stage output changed the partition count). Closes apache#1907
…tion DataFusion 54 plans uncorrelated scalar subqueries as a physical ScalarSubqueryExec wrapping a ScalarSubqueryExpr that reads an in-process shared results container. That container cannot cross process or stage boundaries, and datafusion-proto can only deserialize the expr inside its surrounding exec, so when Ballista splits a plan into stages at shuffle boundaries the executor receives a bare ScalarSubqueryExpr and fails to decode it -- hanging queries such as TPC-H q11/q15/q22. Disable datafusion.optimizer.enable_physical_uncorrelated_scalar_subquery in the Ballista restricted configuration so the optimizer rewrites uncorrelated scalar subqueries to joins, which Ballista distributes correctly. Verified via datafusion-cli that the rewrite produces results identical to the physical-execution path. Closes apache#1909
Add unit tests for the two DataFusion 54 distributed-execution fixes: - restrict_scan_to_partition keeps only the task's own file group, leaves the partition count intact, returns None for an out-of-range partition, and ignores non-file-backed plans. - the Ballista restricted configuration disables datafusion.optimizer.enable_physical_uncorrelated_scalar_subquery so uncorrelated scalar subqueries are rewritten to joins.
DataFusion 54 plans an uncorrelated scalar subquery as a physical ScalarSubqueryExec whose ScalarSubqueryExpr cannot be deserialized once Ballista splits the plan into stages. apache#1909 worked around this by decorrelating such subqueries to joins, which is correct but adds join work for what is logically a constant. Instead, in the client query planner, detect uncorrelated scalar subqueries in the optimized logical plan, run each as its own distributed query, reduce the result to a single value (0 rows -> typed null, 1 row -> the value, more than one -> error), and substitute it as a literal before the outer plan is sent to the scheduler. Nested subqueries are inlined recursively. This supersedes the decorrelation workaround, so the Ballista restricted configuration no longer disables datafusion.optimizer.enable_physical_uncorrelated_scalar_subquery. Closes apache#1910
Reconcile this branch's DataFusion 54 work with the canonical DF54 upgrade (apache#1906) that landed upstream: - Keep enable_physical_uncorrelated_scalar_subquery at DataFusion's default (enabled); this branch materializes uncorrelated scalar subqueries in the client planner instead of disabling the option. - Adopt upstream's higher-order function support in the function registry and TaskContext construction. - Adopt upstream's recursive PhysicalPlanDecodeContext codec. - Adopt upstream's more robust restrict_scan_to_partition that handles MemorySourceConfig and warns on unrecognized DataSourceExec sources. - Drop the make_filter_projection_serde_safe workaround (no longer needed on DF54; upstream added regression coverage in serde).
…ze-scalar-subquery # Conflicts: # ballista/core/src/extension.rs
Convert the collect/substitute unit tests to assert on the plan rendered with display_indent() instead of counts and emptiness, so each test shows the plan it expects. The skip-correlated test now pairs an eligible subquery with a correlated one and checks the collected plan is the eligible one.
milenkovicm
left a comment
There was a problem hiding this comment.
thanks @andygrove just few minor comments
| Default::default(), | ||
| ) | ||
| .await?; | ||
|
|
There was a problem hiding this comment.
can we explicitly disable SET datafusion.optimizer.enable_physical_uncorrelated_scalar_subquery = true, it would give additional context to the test.
There was a problem hiding this comment.
Done — the test now runs SET datafusion.optimizer.enable_physical_uncorrelated_scalar_subquery = true before the query, so it states that it covers the case where the optimizer is free to plan a physical scalar subquery.
|
|
||
| Ok(()) | ||
| } | ||
|
|
There was a problem hiding this comment.
can we also add check with AQE enabled, if it works, if not a jira issue or a test in "unsupported" file?
There was a problem hiding this comment.
Added standalone_aqe / remote_aqe cases (new standalone_context_with_aqe() / remote_context_with_aqe() fixtures set ballista.planner.adaptive.enabled=true). All four cases pass — materialization happens in the client query planner before the plan reaches the scheduler, so it is independent of which distributed planner runs. No issue needed.
…config Set datafusion.optimizer.enable_physical_uncorrelated_scalar_subquery = true in the test so it states the configuration it covers, and add standalone/remote cases with the adaptive planner enabled.
Which issue does this PR close?
Closes #1910.
Rationale for this change
#1909 made uncorrelated scalar subqueries work in distributed execution by
disabling
datafusion.optimizer.enable_physical_uncorrelated_scalar_subquery,which makes the optimizer rewrite them to joins (DataFusion 54's physical
ScalarSubqueryExeccannot be serialized across Ballista's stage boundaries).That is correct but not efficient: an uncorrelated scalar subquery produces a
single value, but rewriting it to a join turns it into a (cross / left) join
against the subquery's one-row result, adding shuffle/build/probe work for what
is logically a constant.
This PR runs the subquery first and substitutes its value, which is both
efficient and avoids the
ScalarSubqueryExecserialization problem entirely.What changes are included in this PR?
BallistaQueryPlanner::create_physical_plan),before sending the plan to the scheduler:
Expr::ScalarSubquerynodes in the optimizedlogical plan (correlated subqueries are left to DataFusion's existing
decorrelation rules);
single
ScalarValue(0 rows → typed null, 1 row → the value, more than onerow → an error);
materialized recursively, innermost first.
enable_physical_uncorrelated_scalar_subquery = falsedefaultadded in DataFusion 54: uncorrelated scalar subqueries (q11/q15/q22) fail — ScalarSubqueryExpr cannot be deserialized in a split stage plan #1909, since the subqueries are now materialized rather than
decorrelated. This supersedes that workaround.
Are there any user-facing changes?
No API or SQL semantics change. Uncorrelated scalar subqueries (e.g. TPC-H
q11/q15/q22) now execute on a Ballista cluster by computing the subquery first
and inlining its value.
Tested with new unit tests for the detection/substitution helpers (including
that correlated subqueries are skipped) and a standalone + remote integration
test that runs
where id = (select max(id) from test)end to end.cargo test,cargo fmt, andcargo clippy --all-targets -- -D warningsare clean.