Skip to content

fix: preserve global limit for multi-partition fetch - #23800

Draft
discord9 wants to merge 1 commit into
apache:mainfrom
discord9:fix/limit-pushdown-global-fetch
Draft

fix: preserve global limit for multi-partition fetch#23800
discord9 wants to merge 1 commit into
apache:mainfrom
discord9:fix/limit-pushdown-global-fetch

Conversation

@discord9

@discord9 discord9 commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

An operator-level fetch on a plan with multiple output partitions is applied to
each partition. It can reduce work, but it does not satisfy a global limit. The
LimitPushdown rule could conflate those scopes and remove the remaining global
enforcement, producing too many rows or composing global and local limits
incorrectly.

What changes are included in this PR?

  • Represent limit propagation with one explicit internal state:
    None, PendingLocal, PendingGlobal, or Enforced (hint-only).
  • Preserve or tighten per-partition fetches as early-stop hints without treating
    them as global limits.
  • Materialize a CoalescePartitionsExec or SortPreservingMergeExec boundary
    before a multi-partition global requirement is considered satisfied.
  • Keep OFFSET and per-partition LocalLimitExec semantics separate from the
    global fetch.
  • Ignore no-op global limits without changing pending limit state.

Are these changes tested?

Yes. Physical optimizer regression tests cover:

  • fetch-capable and unfetchable multi-partition inputs;
  • LIMIT, OFFSET + LIMIT, and OFFSET-only plans;
  • an existing smaller per-partition fetch;
  • Union input fan-out;
  • global limits above multi-partition local limits;
  • no-op global limits; and
  • nested global limits and no-op wrappers;
  • single-output extension combiners hiding multi-partition children;
  • per-partition limits crossing a single-output combiner; and
  • fetch absorption at HashJoin without leaking fetch to its inputs.

The complete physical_optimizer::limit_pushdown integration test module and
workspace clippy were run from the final PR head.

Are there any user-facing changes?

Yes. Queries using these physical plan shapes now preserve the requested global
LIMIT/OFFSET rather than returning too many rows or over-restricting results.
There are no public API changes.

@github-actions github-actions Bot added optimizer Optimizer rules core Core DataFusion crate labels Jul 22, 2026
Signed-off-by: discord9 <discord9@163.com>
@discord9
discord9 force-pushed the fix/limit-pushdown-global-fetch branch from 1229760 to 4b30ece Compare July 28, 2026 10:09
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.99083% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.69%. Comparing base (1b32517) to head (4b30ece).

Files with missing lines Patch % Lines
...atafusion/physical-optimizer/src/limit_pushdown.rs 88.99% 10 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #23800      +/-   ##
==========================================
- Coverage   80.69%   80.69%   -0.01%     
==========================================
  Files        1095     1095              
  Lines      372623   372671      +48     
  Branches   372623   372671      +48     
==========================================
+ Hits       300695   300731      +36     
- Misses      53979    53986       +7     
- Partials    17949    17954       +5     

☔ 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.

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.

LimitPushdown can mistake per-partition fetch for a global limit

2 participants