fix: preserve memory scan fetch during repartitioning - #25068
Closed
discord9 wants to merge 1 commit into
Closed
Conversation
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #25068 +/- ##
========================================
Coverage 81.74% 81.74%
========================================
Files 1128 1128
Lines 416644 416797 +153
Branches 416644 416797 +153
========================================
+ Hits 340585 340712 +127
- Misses 55999 56003 +4
- Partials 20060 20082 +22 ☔ 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. Independently reproduced physical-optimizer correctness fix.
Rationale for this change
Repartitioning a memory source with an existing fetch rebuilds the source without that fetch. A one-partition scan containing batches
[0,1]and[2,3]with fetch 1 returns[0]before optimization, but all four rows after source repartitioning. This also occurs through EnsureRequirements and one/two complete default physical-optimizer passes with a parent filter and no outer limit.The fetch is enforced separately for each original partition. Copying it onto newly formed partitions would still change the result, rather than preserve the original cap.
This is demonstrated for directly constructed physical plans and their optimization; an ordinary SQL-planning reproduction is not established.
What changes are included in this PR?
Have
MemorySourceConfigdecline source-level repartitioning whenever a fetch is present, before either sorted or unsorted repartitioning. The optimizer can still insert an ordinaryRepartitionExecover the existing capped source, distributing rows already admitted by the original caps.This uses the existing optional repartition contract, with no changes to optimizer configuration, statistics, or shared fetch accounting. It is independent of #23800 and #25065.
What is the testing strategy for this PR?
[0,10].[10].[0]and use ordinary round-robin repartition over the original one-partition fetched source.Removing only the guard makes the independently eligible projection/ordering regression fail. The guard was restored byte-exactly; the original direct/API and optimizer reproductions also demonstrated the pre-fix one-row-to-four-row change.
Verified locally:
cargo check -p datafusion-datasourcecargo test -p datafusion-datasource memory:: --lib— 15 passedcargo test -p datafusion --test core_integration physical_optimizer::enforce_distribution::— 88 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 the results of already-capped memory scans during physical optimization. No public API or configuration changes. Source-level repartition is declined for capped sources; ordinary downstream repartition remains available.