Skip to content

fix: preserve file scan fetch during filter pushdown - #25067

Closed
discord9 wants to merge 2 commits into
apache:mainfrom
discord9:fix/preserve-file-scan-fetch
Closed

fix: preserve file scan fetch during filter pushdown#25067
discord9 wants to merge 2 commits into
apache:mainfrom
discord9:fix/preserve-file-scan-fetch

Conversation

@discord9

@discord9 discord9 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

No linked issue. This is an independently reproduced physical-optimizer correctness fix.

Rationale for this change

A new parent predicate must not be moved below an already-enforced file-scan fetch. For an ordered Parquet scan containing [0, 1], Filter(a = 1) → Scan(fetch=1) returns no rows. Filter pushdown currently installs the predicate inside the capped scan, changing the result to [1].

This was reproduced through the physical-plan API, isolated FilterPushdown, and one/two complete default physical-optimizer passes. An ordinary SQL-planning reproduction is not established here.

What changes are included in this PR?

Reject new parent predicates at FileScanConfig::try_pushdown_filters when the scan already has a limit, using the existing unsupported propagation result without updating the source. This preserves existing source predicates and prevents admission through the pruning-only path as well as exact row filtering. Uncapped scans retain their existing filter-pushdown behavior.

The production change is one source-local guard; no new configuration or optimizer machinery. This is independent of #23800 and #25065.

What is the testing strategy for this PR?

Two focused Parquet tests cover:

  • an ordered single-row-group scan with row filtering enabled and statistics pruning disabled, including fetch 1 and 0;
  • original/optimized results and one/two full optimizer passes;
  • uncapped exact filter pushdown;
  • pruning-only predicate admission and preservation of an existing internal scan predicate.

Removing only the guard makes the row-filtering regression return [1] instead of []. The pruning-only ablation fails the no-new-predicate plan assertion; it is not claimed as a separate wrong-row reproduction. The production guard was restored byte-exactly.

Verified locally:

  • cargo check -p datafusion-datasource
  • cargo test -p datafusion --test parquet_integration parquet::filter_pushdown:: — 10 passed
  • cargo fmt --all --check
  • cargo clippy --workspace --all-targets --all-features -- -D warnings

The full workspace runtime test suite was not run.

Are there any user-facing changes?

Preserves results when optimizing an already-capped file scan beneath a parent filter. No public API or configuration changes. This conservatively declines new predicate pushdown at an existing scan limit rather than changing the scan's current predicate semantics.

Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
@github-actions github-actions Bot added core Core DataFusion crate datasource Changes to the datasource crate labels Sep 8, 2026
@discord9 discord9 closed this Sep 8, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.74%. Comparing base (b39cb4a) to head (67f3e65).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #25067      +/-   ##
==========================================
- Coverage   81.74%   81.74%   -0.01%     
==========================================
  Files        1128     1128              
  Lines      416644   416649       +5     
  Branches   416644   416649       +5     
==========================================
- Hits       340585   340584       -1     
- Misses      55999    56005       +6     
  Partials    20060    20060              

☔ 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 datasource Changes to the datasource crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants