fix: preserve global limit for multi-partition fetch - #23800
Open
discord9 wants to merge 10 commits into
Open
Conversation
discord9
force-pushed
the
fix/limit-pushdown-global-fetch
branch
from
July 28, 2026 10:09
1229760 to
4b30ece
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #23800 +/- ##
========================================
Coverage 81.63% 81.63%
========================================
Files 1123 1123
Lines 409729 410016 +287
Branches 409729 410016 +287
========================================
+ Hits 334496 334733 +237
- Misses 55576 55600 +24
- Partials 19657 19683 +26 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
discord9
marked this pull request as ready for review
July 29, 2026 09:33
neilconway
reviewed
Aug 13, 2026
neilconway
left a comment
Contributor
There was a problem hiding this comment.
Thanks for your work on this, @discord9! This PR is overall very good: thorough explanation of the problem, very good test coverage, and well-written implementation. I spent a while working through it and talking to Claude and the basic approach makes sense to me and clears up a clear semantic shortcoming of the current representation.
discord9
force-pushed
the
fix/limit-pushdown-global-fetch
branch
from
August 14, 2026 05:22
37d5ff9 to
c56d310
Compare
discord9
force-pushed
the
fix/limit-pushdown-global-fetch
branch
from
August 25, 2026 08:14
dea219c to
e4cb9a7
Compare
discord9
added a commit
to discord9/datafusion
that referenced
this pull request
Aug 25, 2026
Ports the net change from apache#23800 at e4cb9a7 onto the DataFusion 55 thin fork. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
5 tasks
discord9
force-pushed
the
fix/limit-pushdown-global-fetch
branch
from
September 1, 2026 07:22
e4cb9a7 to
a6aad77
Compare
Signed-off-by: discord9 <discord9@163.com>
Signed-off-by: discord9 <discord9@163.com>
Signed-off-by: discord9 <discord9@163.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
discord9
force-pushed
the
fix/limit-pushdown-global-fetch
branch
from
September 3, 2026 04:16
a6aad77 to
160ec52
Compare
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
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?
Rationale for this change
An operator-level
fetchapplies independently to each output partition, so it cannot always replace a query-wide limit. For example, pushingLIMIT 5into a two-partition scan asfetch=5can return up to 10 rows.The per-partition fetch is still useful as an early-stop hint, but the optimizer must distinguish that hint from a limit that still needs to be enforced. This distinction also affects OFFSET, existing fetches, partition ordering, and operators with multiple children.
What changes are included in this PR?
LimitPushdownnow tracks both the limit values (skipandfetch) and whether a local or global limit is still pending:LocalGlobalThe rule handles the relevant cases as follows.
Partition scope
CoalescePartitionsExecis used when no ordering must be preserved.SortPreservingMergeExecis used when the limit requires the input ordering to remain intact.OFFSET and existing fetches
skip + fetchas an early-stop hint, but a fetch alone never implements OFFSET.existing_fetch <= required_fetch.with_fetchwhen supported; otherwise an explicit limit remains.GlobalLimitExec(skip=0, no fetch) is removed without creating a partition-combining boundary.Unary and custom operators
supports_limit_pushdownmeans that a limit may be delegated through an operator;with_fetchseparately means that the operator can apply a fetch to its own output.with_fetchimplementation can enforce the output bound.supports_limit_pushdownmay pass a pending limit to its single child.with_fetch, it can enforce the bound on its own output. With OFFSET, an explicitGlobalLimitExecis still required above it.Operators with multiple children
A pending limit is not copied independently to every child of an arbitrary multi-child operator, because their combined output could exceed the requested row count.
with_fetchimplementation or an explicit limit node.pendingis cleared and only a safe early-stop fetch is passed to the children.UnionExecis a narrow exception for local limits: each Union output partition comes from one child partition, so the same per-partition local limit can safely be delegated to its children.What is the testing strategy for this PR?
The
physical_optimizer::limit_pushdownintegration tests cover 42 plan shapes, including:with_fetch;Verified on the latest rebased branch with:
Are there any user-facing changes?
Yes. Affected plans now preserve query-wide LIMIT/OFFSET semantics instead of returning too many rows or applying an offset more than once. There are no public API or configuration changes.