Skip to content

feat: union nested Parquet leaves across projection accesses - #24130

Merged
kosiew merged 3 commits into
apache:mainfrom
goutamadwant:gh-24121-union-required-leaves
Aug 16, 2026
Merged

feat: union nested Parquet leaves across projection accesses#24130
kosiew merged 3 commits into
apache:mainfrom
goutamadwant:gh-24121-union-required-leaves

Conversation

@goutamadwant

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

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?

  • Accumulate and deduplicate the leaf offsets required by every narrowing cast on a root column.
  • Add leaves required by get_field accesses to the same union.
  • Derive the Arrow type emitted by the reader from the merged leaf set while retaining a full-read fallback for unsupported partial wrapper shapes.
  • Add coverage for disjoint, overlapping, and repeated casts, cast plus get_field, and casts that diverge below List<Struct>.

Are these changes tested?

Yes. The following checks pass:

  • cargo fmt --all -- --check
  • cargo test -p datafusion-datasource-parquet --lib
  • cargo test --profile=ci --test sqllogictests -- parquet_nested_schema_pruning.slt
  • cargo clippy -p datafusion-datasource-parquet --all-targets --all-features -- -D warnings

The 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.

@codecov-commenter

codecov-commenter commented Aug 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.78313% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.19%. Comparing base (b9399dc) to head (0d33128).

Files with missing lines Patch % Lines
...ion/datasource-parquet/src/projection_read_plan.rs 95.78% 4 Missing ⚠️
...on/datasource-parquet/src/nested_schema_pruning.rs 95.77% 0 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #24130      +/-   ##
==========================================
- Coverage   81.19%   81.19%   -0.01%     
==========================================
  Files        1110     1110              
  Lines      388750   388881     +131     
  Branches   388750   388881     +131     
==========================================
+ Hits       315657   315752      +95     
- Misses      54506    54534      +28     
- Partials    18587    18595       +8     

☔ 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.

@kosiew kosiew left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@goutamadwant,

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(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@github-actions github-actions Bot added the auto detected api change Auto detected API change label Aug 15, 2026
@goutamadwant
goutamadwant force-pushed the gh-24121-union-required-leaves branch from ca91f7b to 0d33128 Compare August 15, 2026 10:08
@github-actions github-actions Bot removed the auto detected api change Auto detected API change label Aug 15, 2026
@kosiew
kosiew added this pull request to the merge queue Aug 16, 2026
@kosiew

kosiew commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Thanks @goutamadwant

Merged via the queue into apache:main with commit bf1c17a Aug 16, 2026
37 checks passed
imtherealnaska pushed a commit to imtherealnaska/datafusion that referenced this pull request Aug 16, 2026
…24130)

## Which issue does this PR close?

- Closes apache#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?

- Accumulate and deduplicate the leaf offsets required by every
narrowing cast on a root column.
- Add leaves required by `get_field` accesses to the same union.
- Derive the Arrow type emitted by the reader from the merged leaf set
while retaining a full-read fallback for unsupported partial wrapper
shapes.
- Add coverage for disjoint, overlapping, and repeated casts, cast plus
`get_field`, and casts that diverge below `List<Struct>`.

## Are these changes tested?

Yes. The following checks pass:

- `cargo fmt --all -- --check`
- `cargo test -p datafusion-datasource-parquet --lib`
- `cargo test --profile=ci --test sqllogictests --
parquet_nested_schema_pruning.slt`
- `cargo clippy -p datafusion-datasource-parquet --all-targets
--all-features -- -D warnings`

The 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.
alamb added a commit to alamb/datafusion that referenced this pull request Aug 20, 2026
branch-55 does not have apache#24130/apache#24315, which taught nested schema pruning
to union the leaves needed by mixed whole-column + field-access reads.
Without that, `select s, s['y'] from narrow` falls back to reading every
physical leaf instead of clipping to the narrow schema, so bytes_scanned
is 219 (matching the unclipped full_schema read) rather than 146.
Dandandan pushed a commit that referenced this pull request Aug 21, 2026
…s adaptation (#24125) (#24530)

## Which issue does this PR close?

- Part of #24462
- Backport of #24125 to `branch-55` (for 55.1.0, tracked in #24462).
- Fixes #24109

## Rationale for this change

With `datafusion.execution.parquet.pushdown_filters = true`, a predicate
on a struct field was reported as fully handled by the scan whenever the
file needed schema adaptation, so `FilterExec` was removed from the plan
and the predicate was silently dropped — returning every row instead of
the filtered set. This is a correctness bug (wrong results), not
specific to 55.0.0, so it fits the backport criteria.

## What changes are included in this PR?

Cherry-pick of #24125 (commit 40c208e).
Git's recursive merge auto-resolved surrounding context differences in
`datafusion/physical-expr-adapter/src/schema_rewriter.rs` and
`datafusion/sqllogictest/test_files/parquet_nested_schema_pruning.slt`;
no manual conflict resolution or adaptation of the fix itself was
required.

One follow-up commit adapts a test expectation: `branch-55` doesn't have
#24130/#24315, which taught nested schema pruning to union the leaves
needed by mixed whole-column + field-access reads (e.g. `select s,
s['y'] from narrow`). Without that optimization, the mixed-access case
falls back to reading every physical leaf, so `bytes_scanned` is `219`
here instead of the `146` the original PR's test expects on `main`. This
is a pre-existing difference in pruning capability, not a correctness
regression from this fix.

## Are these changes tested?

Yes. Carries the original regression coverage, all tests pass.

## Are there any user-facing changes?

`WHERE s['field'] = ...` predicates on struct columns now filter
correctly when Parquet filter pushdown requires schema adaptation. No
API changes.

---------

Co-authored-by: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

datasource Changes to the datasource crate sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Union the required leaves when a nested root is reached by multiple casts (or a cast plus get_field) instead of reading the whole column

3 participants