Skip to content

fix: preserve fetched merges during requirement enforcement - #25065

Draft
discord9 wants to merge 2 commits into
apache:mainfrom
discord9:fix/preserve-distribution-fetch
Draft

discord9 wants to merge 2 commits into
apache:mainfrom
discord9:fix/preserve-distribution-fetch

Conversation

@discord9

@discord9 discord9 commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Related to #14150 and #23800. This fix also applies independently of #23800.

Rationale for this change

Enforcing distribution and sorting requirements can remove a fetch that limits the combined output of several partitions. Reoptimizing a physical plan with a global LIMIT over a partitioned aggregate can therefore return more rows: a reproduced case returns one row after the first pass and three after the second.

For a fetched sort-preserving merge, ordering also determines which rows are selected. Replacing it with a fetched coalesce can return the right number of rows but the wrong values, even if the parent does not require ordered output. Removing it during sort cleanup can leave only partition-local TopK bounds instead of the global limit.

A SQL-derived example is:

SELECT b
FROM (SELECT a, b FROM t ORDER BY a LIMIT 2)
GROUP BY b

With two input partitions containing (a, b) values [(1, 10), (100, 99)] and [(2, 20), (200, 98)], the SQL-generated plan returns [10, 20]. Passing that plan through DefaultPhysicalPlanner::optimize_physical_plan again can remove its fetched merge and return [10, 20, 99]. This reproduces a failure when reoptimizing an existing plan, not a failure of ordinary one-pass SQL execution.

What changes are included in this PR?

  • Preserve fetched operators when removing distribution-changing operators, including fetch zero.
  • Preserve fetched sort-preserving merges and their ordered inputs when replacing order-preserving variants or removing unnecessary sorts.
  • Remove fetch collection and reapplication code made unnecessary by retaining those operators.
  • Update the single-partition merge regression to retain its limit and required input ordering.

Fetch-free distribution and sort-cleanup rewrites remain available.

What is the testing strategy for this PR?

Execution tests cover fetched coalesces, repeated optimization of a partitioned aggregate, ordered TopK selection, OFFSET, partition-local limits, and fetched versus fetch-free merge replacement. Assertions check selected values as well as row counts.

The SQL-derived regression plans and executes both an ordered LIMIT query and the aggregate query above, then reoptimizes each plan through the public physical planner API and checks the results again. Removing the sort-cleanup protection reproduces the extra-row failure; restoring it preserves [10, 20]. Separate deletion tests also reproduced the failures addressed by the distribution protections.

Passed locally:

cargo fmt --all --check
cargo test -p datafusion --test core_integration
cargo test -p datafusion-physical-optimizer --lib
cargo clippy --workspace --all-targets --all-features -- -D warnings

The core integration suite passed 1,126 tests and the physical optimizer suite passed 37 tests.

Are there any user-facing changes?

Distribution and sorting enforcement preserve existing LIMIT/OFFSET bounds and ordered row selection when optimizing physical plans. No public API or configuration changes.

Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.74%. Comparing base (8a92281) to head (b50c877).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #25065      +/-   ##
==========================================
+ Coverage   81.72%   81.74%   +0.02%     
==========================================
  Files        1127     1128       +1     
  Lines      416519   416633     +114     
  Branches   416519   416633     +114     
==========================================
+ Hits       340401   340587     +186     
+ Misses      56115    55988     -127     
- Partials    20003    20058      +55     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
@discord9 discord9 changed the title fix: preserve fetched merges during distribution enforcement fix: preserve fetched merges during requirement enforcement Sep 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate optimizer Optimizer rules

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants