fix: preserve file scan fetch during filter pushdown - #25067
Closed
discord9 wants to merge 2 commits into
Closed
Conversation
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 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. 🚀 New features to boost your workflow:
|
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?
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_filterswhen 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:
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-datasourcecargo test -p datafusion --test parquet_integration parquet::filter_pushdown::— 10 passedcargo fmt --all --checkcargo clippy --workspace --all-targets --all-features -- -D warningsThe 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.