feat: union nested Parquet leaves across projection accesses - #24130
feat: union nested Parquet leaves across projection accesses#24130goutamadwant wants to merge 3 commits into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #24130 +/- ##
========================================
Coverage 81.06% 81.06%
========================================
Files 1107 1107
Lines 382191 382300 +109
Branches 382191 382300 +109
========================================
+ Hits 309807 309904 +97
- Misses 54082 54090 +8
- Partials 18302 18306 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
kosiew
left a comment
There was a problem hiding this comment.
Thanks for working on this. The leaf union approach looks solid, and the added coverage for multiple casts, cast plus get_field, and nested list structs is helpful.
I only have one small non-blocking suggestion to make the fallback behavior a bit more explicit in the tests.
| /// projection is supported through the same struct, list, and large-list | ||
| /// shapes as [`clip_for_cast`]. Any partial selection below another wrapper | ||
| /// returns `None`, preserving the total-fallback property of cast clipping. | ||
| pub(crate) fn type_for_leaf_subset( |
There was a problem hiding this comment.
Could we add a small unit test for the case where the union covers every leaf? type_for_leaf_subset intentionally returns None there so the caller falls back to the full root. It would be nice to pin that behavior explicitly and avoid a future change treating a full union as an unsupported partial projection.
Which issue does this PR close?
get_field) instead of reading the whole column #24121.Rationale for this change
When one projected nested column is consumed through multiple narrowing casts, or through both a narrowing cast and
get_field, the Parquet read plan currently falls back to reading the entire root column. The leaves required by those consumers can be combined, so reading the full root performs unnecessary I/O.What changes are included in this PR?
get_fieldaccesses to the same union.get_field, and casts that diverge belowList<Struct>.Are these changes tested?
Yes. The following checks pass:
cargo fmt --all -- --checkcargo test -p datafusion-datasource-parquet --libcargo test --profile=ci --test sqllogictests -- parquet_nested_schema_pruning.sltcargo clippy -p datafusion-datasource-parquet --all-targets --all-features -- -D warningsThe SQL logic test also verifies that the merged projections return the expected values and scan fewer bytes than a full-root read.
Are there any user-facing changes?
No API or SQL behavior changes. Queries with multiple nested accesses to the same Parquet root can read fewer leaf columns.