fix: [DO NOT MERGE] Validate nested Parquet leaf pruning against datafusion#23391#4866
Draft
mbutrovich wants to merge 1 commit into
Draft
fix: [DO NOT MERGE] Validate nested Parquet leaf pruning against datafusion#23391#4866mbutrovich wants to merge 1 commit into
mbutrovich wants to merge 1 commit into
Conversation
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?
Relates to #4859. (Draft: it validates the fix end to end but should not merge until the DataFusion change is released. See below.)
Rationale for this change
Comet's native Parquet scan read entire nested columns even when Spark's plan showed a pruned nested
ReadSchema, so a query needing one leaf of a widearray<struct<...>>fetched and decoded every leaf (#4859 measured ~1.35 TB vs plain Spark's ~30.9 GB for the same files and rows).The root cause is in DataFusion, not Comet. Spark's
SchemaPruningbakes nested pruning into the scan'srequiredSchemaas a narrower type, and Comet already forwards that pruned schema. DataFusion's projection-mask derivation only pruned nested leaves forget_fieldexpression chains and expanded any other projection (including the whole-column cast the schema adapter inserts for a narrowed nested type) to every leaf of the top-level column. So the pruning was correct in the output but not in the I/O.The fix is upstream in apache/datafusion#23391, which derives a leaf-level
ProjectionMaskwhen a projected column's requested type is a nested subtree of the file's type.What changes are included in this PR?
No Comet production code changes. Comet already forwards the pruned schema, so the fix is entirely on the DataFusion side.
This draft points the DataFusion dependencies at a fork branch that is DataFusion 54.0.0 plus the fix (via
[patch.crates-io]innative/Cargo.toml, with the regeneratednative/Cargo.lock), so CI can exercise the change end to end. That patch and lock are temporary and will be replaced by a normal DataFusion version bump once the fix lands in a release, at which point this closes #4859. Not for merge as-is.It also adds regression tests to
CometNativeReaderSuite.How are these changes tested?
New tests in
CometNativeReaderSuitewrite anarray<struct<...>>(and a deeperarray<struct<struct<...>>>) where a high-entropy leaf dominates the column size, then assertCometNativeScanExec'sbytes_scannedwhen projecting only the small leaf is a fraction of projecting the large leaf too. Before the fix these were equal (whole column read).Locally,
CometFuzzTestSuiteandCometNativeReaderSuitepass against the fork branch, and this PR runs full CI.