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:
|
5 tasks
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
5 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 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:
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 throughDefaultPhysicalPlanner::optimize_physical_planagain 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?
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:
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.