fix: preserve fetched merges during distribution enforcement - #25065
Draft
discord9 wants to merge 1 commit into
Draft
fix: preserve fetched merges during distribution enforcement#25065discord9 wants to merge 1 commit into
discord9 wants to merge 1 commit into
Conversation
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
This was referenced Sep 8, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. 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. 🚀 New features to boost your workflow:
|
6 tasks
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?
Related to #14150 and #23800. This is independently reproducible on main and does not depend on #23800.
Rationale for this change
Running the default physical optimizer a second time can change query results. On main
8a9228164fddeaa03c9ee510b641990e403f2eb5, a global LIMIT over a partitioned aggregate returns one row after the first optimization and three after the second: distribution enforcement removes theCoalescePartitionsExec(fetch=1)that limits the combined output.Fetched sort-preserving merges have an additional requirement: their ordering determines which rows survive. Replacing
SortPreservingMergeExec(fetch=N)withCoalescePartitionsExec(fetch=N)can preserve the row count while selecting the wrong rows, even when the parent does not require ordered output.What changes are included in this PR?
fetch=0. Preserve the actual global row-selection boundary rather than moving its numeric fetch elsewhere.What is the testing strategy for this PR?
Native execution regressions cover:
Baseline execution on unmodified main (
8a9228164f) and the latest #23800 head (a403530d99) demonstrated the global LIMIT regression through two complete default physical optimizer passes: one row on the first pass, three on the second.Independent guard-removal experiments on #23800 also verified both protections:
[10,20,98,99]instead of[10,20].[10,99]instead of[10,20].On the main-based branch, the following checks pass:
The fix combined with #23800 additionally passes 87 distribution tests, 42 LimitPushdown tests, 30 EnsureRequirements tests, and strict all-target/all-feature Clippy for
datafusionanddatafusion-physical-optimizer.Are there any user-facing changes?
Queries retain their global LIMIT/OFFSET and ordered TopK selection when physical plans are optimized again. No public API or configuration changes.