Repository navigation
Conversation
Pin the datafusion crates to apache/datafusion@611311cd7 and bump the dependencies it requires (arrow 60, object_store 0.14, rustyline 19, MSRV 1.95). Port to the upstream API changes and refresh the plan snapshots and TPC-H golden files.
andygrove
marked this pull request as draft
October 30, 2026 22:31
andygrove
marked this pull request as ready for review
October 30, 2026 22:31
apache#2613 moved FailingPlanRewriteExec into test_utils and dropped the test module's `ExecutionPlan` import with it, but the cache-reader test that apache#2633 added still names the trait. Each change passed CI alone; together the scheduler's test target no longer compiles on main.
DataFusion now plans `SHOW TABLES`, `SHOW COLUMNS`, `SHOW CREATE TABLE`, `SHOW ALL` and `DESCRIBE` as `LogicalPlan::Show` instead of rewriting them into `information_schema` queries. The Flight SQL frontend only recognised the latter as catalog reads, so it distributed a `Show` plan. Its streaming table cannot be serialized by the physical codec, so the job failed on the cluster (`catalog_queries_are_answered_without_distributing_them`). Classify `Show` as RunOnScheduler, as BallistaQueryPlanner already does on the client, and cover both the classification and the scheduler-side answer with unit tests that need no socket.
The workspace is pinned to a DataFusion commit on main, and the Python crate took the Ballista crates by path, so its build pulled that DataFusion in next to the DataFusion 55 that datafusion-python needs, and python/Cargo.lock fell out of date with the manifests. The published 55.0.0 crates cannot stand in: the Python client uses core changes made after that release (apache#2588). Take the Ballista crates from 72d6807, the last commit on main before the pin, and keep the path dependencies commented for when the workspace is back on a released DataFusion.
The Python workflow lints the whole repository and runs on any change under python/, so these files from apache#2697 fail its `ruff format --check` (ruff 0.14.13) for every pull request that touches python/. Pure reformatting: the parsed syntax tree of every file is unchanged.
andygrove
marked this pull request as draft
October 31, 2026 18:49
DataFusion now runs its sanity check after the last physical optimizer rule, including custom chains, and has no way to turn it off. Several Ballista chains ended with plans that only became valid on a later replan. - End the AQE plan preparation rules with EnsureRequirements. - Have ParallelWindowRule repair parents whose distribution requirement the K-partition rewrite no longer meets, and sort NULLS FIRST windows to their own null placement instead of relying on a later replan. - Build the join selection test plans from valid inputs. - Replace the deprecated supports_limit_pushdown in ChaosExec.
# Conflicts: # benchmarks/cluster/run.py
Enforce requirements only after the preparation rules of a plan that comes from the logical planner. A submitted physical plan carries repartitions the client placed on purpose, and EnsureRequirements would drop the ones it finds unnecessary before they are pinned as exchanges.
andygrove
marked this pull request as ready for review
November 4, 2026 14:38
Contributor
|
Would it be ok to postpone this until I get two python PRs merged if not very urgent ? |
Member
Author
Yes, of course. |
Contributor
|
thanks for understanding, will try to address few of your comments later today |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
No issue. This is preparation for the DataFusion 56 upgrade, following the same approach used for DataFusion 55 (#2241).
Rationale for this change
Tracking DataFusion's branch-56 ahead of the 56 release lets us absorb upstream breaking changes in small steps instead of all at once when 56 ships. Pinning to an exact commit keeps builds reproducible, and the pin can be bumped periodically until the release, when we switch back to crates.io versions.
What changes are included in this PR?
Dependencies:
datafusion,datafusion-cli,datafusion-functions-aggregate-common,datafusion-proto,datafusion-proto-commonanddatafusion-substraitto apache/datafusion@1635ed9.flight-sql-experimentalfeature is renamedflight-sql), object_store 0.14.2, and rustyline 19 inballista-cli.datafusion.protoanddatafusion_common.protowithdev/update_datafusion_proto.py.Ports to upstream API changes:
LogicalPlan::DescribeTableis replaced byLogicalPlan::Show, which now coversDESCRIBE,SHOW TABLES,SHOW COLUMNS,SHOW CREATE TABLE,SHOW ALLandSHOW FUNCTIONS. These all read the client's catalog and cannot be serialized, soBallistaQueryPlannerplans everyShowlocally. The Flight SQL frontend likewise runsShowstatements on the scheduler, against the session catalog it holds. PreviouslySHOW TABLESand friends were rewritten intoinformation_schemaqueries, which both already kept off the cluster.OptimizerRule::supports_rewritewas removed upstream.ExecutionPlan::with_preserve_orderis deprecated and no longer called, soChaosExecstops overriding it.TableProvider::scantakesOption<&[usize]>for the projection.DataFusion's final physical plan check:
SanityCheckPlanafter the last physical optimizer rule of every pipeline, including custom ones, and offers no way to turn it off. Several AQE chains used to end with plans that only became valid on a later replan.AdaptivePlanner::try_newwithEnsureRequirements. A submitted physical plan skips it, because the client placed its repartitions on purpose andEnsureRequirementswould drop the ones it finds unnecessary before they are pinned as exchanges.ParallelWindowRuleleaves K partitions where the window had one, which broke any parent that required a single partition or a key partitioning. The rule now inserts a coalesce, an order-preserving merge or a hash repartition for such parents. ANULLS FIRSTwindow also gets a per-partition sort to its own null placement, since the range sketch always sorts the keyNULLS LAST. The replan used to add that sort.ChaosExecimplementslimit_behavior, becausesupports_limit_pushdownis deprecated.Test updates:
EnsureRequirements, because the unsorted form is rejected, and the nested-join test plans hash joins instead.HashJoinExecinPartitionedmode. Ballista's planner and AQE already lower null-aware joins toCollectLeft, so only two join-selection tests that built thePartitionedform needed to change.RANGEoffsets on aUtf8order key, so the parallel-window "no encoding" test uses aCURRENT ROWframe instead. It still fails if the key is switched to an encodable type.DiskManagernow creates its temp directories on the first spill file, so the spill-dir test creates one before checking the paths.InterleaveExecwith non-interleavable children no longer errors upstream, so a precondition check in theNormalizeInterleaveRuletest is dropped. The rule itself may no longer be needed, which can be a follow-up.SortMergeJoinExecinstead of placing aProjectionExecabove it, and the byte-size statistics estimates changed. TheEXPLAIN ANALYZEexpectation picks up newAggregateExecmetrics.SHOWstatements with unit tests that need no socket: which statements run on the scheduler, and thatSHOW TABLESlists the session's tables.Python bindings:
python/stays on DataFusion 55, because it builds against a datafusion-python release. It takes the workspace crates by path, which would put the pinned DataFusion into its build next to DataFusion 55, so itsballista*dependencies now point at72d68073onmain(from before this pin) andpython/Cargo.lockis updated to match.python/srcuses core changes made after that release (feat(python): Plug into DataFusion Python natively through a custom FFI query planner #2588).python/Cargo.toml, once DataFusion 56 and datafusion-python 56 are released.Fixes for CI that are not part of the upgrade:
ExecutionPlanin theexecution_graphtests. fix(scheduler): keep AQE stage in the graph when a stage transition fails #2613 moved theFailingPlanRewriteExectest helper out of that test module and dropped the import with it, but the test that feat: support DataFrame::cache() materialize and read-back #2633 added still needs it, so the scheduler's test target does not compile onmain. The same one-line fix is in feat: let custom ExecutionPlan nodes restrict themselves to a task's partitions #2721 and feat: let hash shuffles place rows by a client-computed partition id #2723, so whichever merges first makes the others no-ops.ruff formatoverbenchmarks/cluster/(from feat(bench): scripts to run the Ballista, Spark, Comet and Trino comparison on Kubernetes #2697). The Python workflow lints the whole repository and runs on any change underpython/, so these unformatted scripts fail it for any PR that touchespython/. Formatting only: the syntax tree of every file is unchanged.Are there any user-facing changes?
Downstream crates now need arrow 60, object_store 0.14 and Rust 1.95, and see the DataFusion 56 API (for example
LogicalPlan::Showand the newTableProvider::scansignature).EXPLAINnow shows the repartitions in itsphysical_planrow, which used to be omitted. Labelledapi-changefor that reason. The Python client is unaffected and stays on DataFusion 55.