fix: apply struct field filters when the file schema needs adaptation by adriangb · Pull Request #24125 · apache/datafusion

adriangb · 2026-08-05T19:58:21Z

Which issue does this PR close?

Closes Parquet filter pushdown silently drops a get_field predicate when the file needs schema adaptation (wrong results) #24109.

Rationale for this change

With datafusion.execution.parquet.pushdown_filters = true, a filter on a struct field returns all rows when the declared table schema differs from the physical file schema for that column:

-- file stores s as Struct<x: Int32>, table declares Struct<x: BIGINT>
SELECT id, s['x'] FROM t WHERE s['x'] = 200;
-- returns 3 rows instead of 1

The planning-time decision and the runtime construction disagree:

ParquetSource::try_pushdown_filters evaluates can_expr_be_pushed_down_with_schemas against the table schema. get_field(s, 'x') has a bare column under the get_field, so it reports the predicate as fully handled and FilterExec is removed from the plan.
At open time the expression adapter rewrites the predicate against the file schema. Because the struct types differ, rewrite_column wraps the whole column in a cast, giving get_field(cast(s AS Struct<x: Int64>), 'x').
PushdownChecker only recognizes get_field whose first argument is a Column. It now sees a CastExpr, falls through to normal traversal, hits the struct Column, and rejects pushdown — so no row filter is built and the conjunct is silently dropped.

Nothing applies the predicate, and the scan returns unfiltered rows.

What changes are included in this PR?

Narrow the cast to the field that is actually read, in DefaultPhysicalExprAdapter:

get_field(cast(s AS Struct<x: Int64>), 'x')  ->  cast(get_field(s, 'x') AS Int64)

Expressions are rewritten bottom-up, so the new try_narrow_struct_cast matches the get_field node after its struct argument has already been wrapped, and rebuilds the get_field over the uncast struct (recomputing its return field from the physical field type) with the cast moved outside. This keeps the column visible under the get_field, so the Parquet row filter builder makes good on what planning promised.

Two details worth calling out:

A field that is missing from the file collapses to a typed null literal, matching what the struct cast would have produced (DataFusion's struct casts match by name and fill missing target fields with nulls).
get_field on a Map column is a runtime key lookup rather than a schema-level field access, so map values keep the whole-column cast.

As a side effect this also avoids materializing an entire cast struct just to read one field, which is a small win for any struct-field access over an evolved schema — not only for filters.

Not addressed here

The issue also raises the broader concern that "a static determination made at planning time about what the scan can do, and the runtime construction that has to make good on it, are computed by different code against different schemas, and there is no mechanism forcing them to agree." This PR fixes the reported wrong-results bug; it does not add a mechanism (e.g. post-decode filtering in ParquetOpener) that would make any future divergence safe by construction. That seems worth doing separately.

Are these changes tested?

Yes.

datafusion/physical-expr-adapter/src/schema_rewriter.rs: unit tests for the narrowed cast (flat and nested field access), the missing-field null literal, and that Map columns keep their cast.
datafusion/datasource-parquet/src/opener/mod.rs: end-to-end opener tests reading a Struct<x: Int32> file through a Struct<x: Int64> table schema with pushdown enabled, plus a matching-schema control.
datafusion/sqllogictest/test_files/parquet_filter_pushdown.slt: a SQL-level regression test. Verified that it fails on main (returns all 3 rows) and passes with the fix.

Full runs: cargo clippy --all-targets --all-features -- -D warnings, the complete sqllogictest suite (498 files), datafusion-physical-expr-adapter, datafusion-datasource-parquet, and the datafusion core_integration / parquet_integration suites all pass.

Are there any user-facing changes?

A wrong-results bug fix: struct-field predicates are now applied when the scan needs schema adaptation. No public API changes.

Generated by Claude Code

adriangb · 2026-08-05T19:58:46Z

+    /// Filters on struct fields (`s['x'] = 200`) must still be applied when the
+    /// table schema disagrees with the physical file schema, which forces the
+    /// expression adapter to insert a cast.
+    ///
+    /// See <https://github.com/apache/datafusion/issues/24109>.
+    mod struct_field_pushdown {


Prefer SLT tests if possible

Copilot

Pull request overview

Fixes a wrong-results bug in Parquet filter pushdown when struct-field predicates are used and the declared table schema differs from the physical file schema. The change ensures the runtime schema adaptation preserves a get_field(Column(..), ...) shape so Parquet’s row-filter builder can still recognize and apply the predicate that planning-time pushdown claimed would be handled.

Changes:

Add a physical-expr rewrite that narrows cast(struct) under get_field into cast(get_field(..)), preserving pushdown compatibility and avoiding casting whole structs unnecessarily.
Add unit tests covering flat, nested, and flattened multi-key get_field paths, missing-field-to-typed-null behavior, and Map behavior (no narrowing).
Add a SQL logic regression test reproducing issue #24109 and validating correct filtering for both schema-adapted and matching-schema reads.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
datafusion/physical-expr-adapter/src/schema_rewriter.rs	Introduces `try_narrow_struct_cast` + field-path resolution and adds focused unit tests for the new rewrite behavior.
datafusion/sqllogictest/test_files/parquet_filter_pushdown.slt	Adds an end-to-end SQL regression test to prevent the struct-field predicate from being silently dropped under schema adaptation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

codecov-commenter · 2026-08-05T20:46:10Z

Codecov Report

❌ Patch coverage is 96.83908% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.25%. Comparing base (c429919) to head (6081cc1).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...usion/physical-expr-adapter/src/schema_rewriter.rs	96.83%	7 Missing and 4 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #24125      +/-   ##
==========================================
+ Coverage   81.24%   81.25%   +0.01%     
==========================================
  Files        1113     1113              
  Lines      392744   393092     +348     
  Branches   392744   393092     +348     
==========================================
+ Hits       319090   319423     +333     
- Misses      54900    54906       +6     
- Partials    18754    18763       +9

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

zhuqi-lucas · 2026-08-06T15:23:52Z

+/// nested struct fields.
+fn resolve_field_path<'a>(fields: &'a Fields, path: &[&str]) -> FieldPathResolution<'a> {
+    let Some((field_name, rest)) = path.split_first() else {
+        return FieldPathResolution::NotAStruct;


Minor: an empty path here returns NotAStruct, which reads a little oddly — an empty path is not really "not a struct". It is unreachable given the non-empty field_name_exprs guard in try_narrow_struct_cast, so a one-line comment noting it is a defensive default would save the next reader a double-take.

Good point. I fixed it by taking the first path as it's own parameter, making the state unrepresentable.

zhuqi-lucas · 2026-08-06T15:23:54Z

        Ok(Transformed::no(expr))
    }

+    /// Rewrite `get_field(cast(s AS Struct<..>), 'f')` into


Nice fix. One design question for the record: did you consider teaching the pushdown side (PushdownChecker / row-filter builder) to see through the cast — i.e. recognize get_field(cast(col), 'f') — instead of narrowing it here?

I assume narrowing was chosen because (a) it avoids materializing the whole cast struct just to read one field, and (b) it fixes it at the source, so every consumer that pattern-matches get_field(column, 'f') benefits — not just the row filter — rather than loosening the pushdown contract to see through arbitrary casts. Worth capturing that rationale.

Relatedly, the PR notes the broader planning-vs-runtime schema divergence is intentionally out of scope — a tracking issue for the "safe by construction" mechanism (e.g. post-decode filtering in ParquetOpener) would be good so it is not lost. Happy to file it.

did you consider teaching the pushdown side (PushdownChecker / row-filter builder) to see through the cast — i.e. recognize get_field(cast(col), 'f') — instead of narrowing it here?

I agree that this fix seems very specific -- it almost seems like it is focusing on the symptom rather than the underlying problem (the physical / logical mismatch that @zhuqi-lucas is pointing out)

It seems like if this is a rewrite that should be done, shouldn't we be doing at a higher level 🤔

Both of your intuitions is right. The major source of fiction right now is that we have to decide to accept filters before seeing the physical schema, but then apply them against the physical schema. Thus situations like this bug arise where a cast must be introduced but that changes our ability to evaluate the filter or not. Today that causes a correctness bug. The solution to turn this from a correctness problem into a performance optimization problem is #22384.

That said I'll look into the suggestion and see if there's an alternative implementation for this PR.

I see -- so the general case solution is that we need some way to fall back apply a filter outside of the arrow-rs parquet decoder in some cases (if we can't push a predicate into the decoder due to physical schema mismatch)?

If so that also sounds similar to the idea of dynamically switching from pushed down filters to filter after the scan 🤔

yes it’s all the same (complex) problem :(

It's a web of:

This is a bug, the lasting fix that makes this a perf improvement and not a correctness issue is to fall back to running filters after the projection (FilterExec embedded in Parquet scan, can never fail).

That work is blocked because of perf regressions, a lot of them related to HashJoinExec dynamic filters (they go from never being evaluated anywhere to being evaluated, and they are often regressions)

I might try to propose disabling the hash join dynamic filters by default.

But I still think we should merge this PR in the general shape it's in.

alamb · 2026-08-06T20:55:08Z


 # Mixed access -- the whole (narrowed) column and a subfield of it -- still
-# reads only the narrow schema's leaves.
+# reads only the narrow schema's leaves. The whole-column read goes through


I am not sure what this comment is trying to say -- it seems like it is trying to explain implementation details. I think we can remove it from here unless it is adding crititcal context past this PR

alamb · 2026-08-06T20:56:38Z

-# more precise, single-leaf pushdown path.
+# `get_field` on a schema-narrowed struct is rewritten to
+# `CAST(get_field(s, 'x'))` rather than `get_field(CAST(s), 'x')`, so it takes
+# `get_field`'s own single-leaf pushdown path: the read clips all the way down


I am not sure what 'get_fields own single-leaf pushdown path is' (It seems like a bunch of implementation detail -- can we just clarify that this query should read fewer bytes because it is selecting a field of s (not all the fields) ?

alamb · 2026-08-06T20:57:24Z

+##########
+# Regression test for https://github.com/apache/datafusion/issues/24109
+#
+# When the declared table schema differs from the physical file schema, the


I think the old behavior is not very relevant after this PR -- maybe this could just focus on what this case covers-- namely that the declared table schema differs from the physical schema

alamb · 2026-08-06T20:58:31Z

+LOCATION 'test_files/scratch/parquet_filter_pushdown/struct_schema_cast.parquet';
+
+query II
+SELECT id, s['x'] FROM t_struct_no_schema_cast WHERE s['x'] = 200;


can we also run the query too:

SELECT id, s['x'] FROM t_struct_schema_cast WHERE s['x'] > 100 AND id > 2;

alamb · 2026-08-06T20:59:12Z

+LOCATION 'test_files/scratch/parquet_filter_pushdown/struct_schema_cast.parquet';
+
+query II
+SELECT id, s['x'] FROM t_struct_missing_field WHERE s['missing'] = 200;


how about also running the two queries above and verifying that they still get the right answer even when there are new fields inserted?

alamb · 2026-08-06T21:00:31Z

+        (logical, physical)
+    }
+
+    /// `s['x']` where the file stores `x` as `Int32` and the table declares


do these unit tests add any additional coverage compared with the .slt coverage? I think the slt coverage is adequate and we could remove these tests and make the PR much smaller

alamb · 2026-08-06T21:01:28Z

+            vec![Field::new("x", DataType::Int64, true)],
+        );
+
+        let adapter = DefaultPhysicalExprAdapterFactory


there is a lot of boiler plate here (factor creation rewrite, cast, etc) -- maybe it could be factored into a helper so it is clearer what is being tested and what is setup

alamb · 2026-08-06T21:03:03Z

    }

+    /// Rewrite `get_field(cast(s AS Struct<..>), 'f')` into
+    /// `cast(get_field(s, 'f') AS <type of f>)`.


Can we also describe here the rationale for why we would want to do this rewrite? It is not obvious I think from these commens

Added below:

/// Narrowing that cast is worthwhile for two reasons: /// /// 1. Reading one field should not cost a whole struct. The wide form /// casts every field of the column — including ones the query never /// reads — to produce a value that is immediately discarded except for /// one field. /// 2. It keeps the column visible. Consumers throughout the codebase /// pattern match on `get_field(column, 'f')` to recognise a struct /// field access; a cast between the `get_field` and its column defeats /// that match, and each such consumer then falls back to whatever it /// does for an unrecognised expression.

alamb · 2026-08-06T21:04:00Z

+    /// by [`Self::rewrite_column`] whenever the logical and physical struct
+    /// types differ. Casting the whole struct just to read one field is
+    /// wasteful, and — more importantly — it hides the underlying column from
+    /// consumers that pattern match on `get_field(column, 'f')`. The Parquet


the silently dropping the predicate part I think is an implementation detail that may not be relevant in the future.

alamb · 2026-08-06T21:05:33Z

        Ok(Transformed::no(expr))
    }

+    /// Rewrite `get_field(cast(s AS Struct<..>), 'f')` into


did you consider teaching the pushdown side (PushdownChecker / row-filter builder) to see through the cast — i.e. recognize get_field(cast(col), 'f') — instead of narrowing it here?

I agree that this fix seems very specific -- it almost seems like it is focusing on the symptom rather than the underlying problem (the physical / logical mismatch that @zhuqi-lucas is pointing out)

It seems like if this is a rewrite that should be done, shouldn't we be doing at a higher level 🤔

Addresses review feedback on apache#24125: - Say *why* the cast is narrowed (reading one field should not cost a whole struct; keeping the column visible keeps every `get_field(column, 'f')` consumer working) and why it is fixed in the adapter rather than by teaching one consumer to see through casts. - Drop the description of the pre-fix row-filter behaviour, which is an implementation detail that will date. - Note that the empty-path arm of `resolve_field_path` is a defensive default, not a claim about empty paths. - Run the compound filter against the matching-schema table too, and check that declaring a field the file lacks leaves the fields it does have answering correctly. - Trim the sqllogictest comments to what the queries demonstrate rather than how the read plan gets there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MebN5PsVnYvXUeVKju5K7P

timsaucer · 2026-08-11T18:28:22Z

It looks like this is one of the blockers for the release of 55.0.0. Commenting so I get notifications about the status of this PR.

adriangb · 2026-08-11T18:33:24Z

@alamb @timsaucer since this has been present since 54 (#24109 (comment)) I don't think this should block the 55 release.

alamb · 2026-08-11T18:45:37Z

@alamb @timsaucer since this has been present since 54 (#24109 (comment)) I don't think this should block the 55 release.

Yea I agree though it would be nice. I will take another look at this one

alamb · 2026-08-11T18:45:49Z

(we can always add it to a 55.1.0 release too)

When the declared table schema differs from the physical file schema for a struct column, the expression adapter wraps the whole struct column in a cast, so `s['x']` becomes `get_field(cast(s AS Struct<..>), 'x')`. That hides the column from consumers that pattern match on `get_field(column, 'f')`. The Parquet scan is one such consumer: it decides at planning time (against the table schema) that a struct-field predicate can be evaluated as a row filter and reports it as fully handled, so `FilterExec` is removed from the plan. At runtime the row filter builder no longer recognizes the adapted expression, silently drops the predicate, and the query returns unfiltered rows. Narrow the cast to the field that is actually read: `get_field(cast(s AS Struct<..>), 'x')` becomes `cast(get_field(s, 'x') AS <type of x>)`. This keeps the column visible under the `get_field`, and also avoids materializing a whole cast struct just to read one field. Fields that are missing from the file collapse to a typed null literal, matching what the struct cast would have produced. `get_field` on a Map column is a runtime key lookup rather than a schema-level field access, so those keep the whole-column cast. Closes apache#24109. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MebN5PsVnYvXUeVKju5K7P

`get_field` has a flattened multi-key form: the logical simplifier rewrites `s['a']['b']` into `get_field(s, 'a', 'b')`. The narrowing rule only matched the two-argument form, so nested field access kept the whole-struct cast and stayed exposed to the wrong-results bug it was meant to fix. Resolve the full key path through nested struct fields on both the logical (cast target) and physical sides, and rebuild `get_field` with every key preserved. A path whose leaf is missing from the file still collapses to a typed null literal; a path that runs through a non-struct field is left alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MebN5PsVnYvXUeVKju5K7P

Drop the opener-level Rust tests in favour of SLT, which covers the same ground end to end through the planner. Adds a matching-schema control and a missing-field case alongside the existing adapted-schema tests, so the deleted Rust coverage is preserved. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MebN5PsVnYvXUeVKju5K7P

…rowing Coverage analysis of the narrowing showed two reachable branches with no test behind them: a struct column that needs no adaptation at all, and one where only a sibling field forced the column-level cast, so the accessed field needs no cast of its own. Both matter — the first is the common case the rewrite must not disturb, the second is where the cast disappears rather than moving. The remaining uncovered branches in the function are guards against shapes that cannot reach it: a `get_field` with fewer than two arguments, and any key path running through a non-struct field, which the column-level cast validation rejects first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MebN5PsVnYvXUeVKju5K7P

Narrowing the struct cast makes `DefaultPhysicalExprAdapter` produce a shape `build_read_plan_with_cast_clipping` did not handle at the time this PR was written: `SELECT s, s['y']` over a narrowed struct reads the whole column through the cast and the field through a `get_field` on the bare column, and a root carrying both access kinds fell back to reading every physical leaf. apache#24315 has since generalized that function to keep the union of the leaves both access kinds need, so the code fix this commit originally carried is no longer necessary; only the test expectations remain. `select s['x'] from narrow` now clips all the way down to `x` (146 -> 75 bytes) because the field access no longer hides behind a whole-struct cast, and `select s, s['y'] from narrow` still reads only the narrow leaves. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Addresses review feedback on apache#24125: - Say *why* the cast is narrowed (reading one field should not cost a whole struct; keeping the column visible keeps every `get_field(column, 'f')` consumer working) and why it is fixed in the adapter rather than by teaching one consumer to see through casts. - Drop the description of the pre-fix row-filter behaviour, which is an implementation detail that will date. - Note that the empty-path arm of `resolve_field_path` is a defensive default, not a claim about empty paths. - Run the compound filter against the matching-schema table too, and check that declaring a field the file lacks leaves the fields it does have answering correctly. - Trim the sqllogictest comments to what the queries demonstrate rather than how the read plan gets there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MebN5PsVnYvXUeVKju5K7P

`resolve_field_path` had to handle an empty key path even though callers guarantee at least one key, and returned `NotAStruct` for it — which is not what that variant means. Take the first key as its own parameter so the invariant is carried by the signature and there is no empty path to resolve. The redundant `field_name_exprs.is_empty()` guard at the call site goes away with it, subsumed by the `split_first` the new signature requires. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

zhuqi-lucas

Went back over the narrowing logic and it holds up: the physical/logical type handling is correct — returns the narrowed get_field directly when the physical and logical fields are equal, and otherwise wraps it in a per-field CastExpr with the original struct-cast's options, so the value matches "cast the whole struct then extract" while only touching the one field. Missing physical field → a typed null literal matches struct-cast fill semantics, and the empty-path / non-struct cases bail cleanly.

LGTM as a targeted fix; agree the general fix is the post-projection filter fallback tracked separately, we can add a follow-up for this.

alamb · 2026-08-20T17:58:23Z

Thank you for reviewing this @zhuqi-lucas and for the fix @adriangb 🙏

…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>

…he#24680) ## Which issue does this PR close? Closes apache#24679. Follow-up to apache#24125 and apache#24530. ## Rationale for this change When Parquet files have different schemas, DataFusion may need to convert a file's Struct column to the table's logical Struct type. If a query only reads `s.x`, struct-cast narrowing can save work by converting just `x` instead of every field in `s`. That optimization must preserve the meaning of the original expression. The earlier narrowing rule could hide an explicit cast's error, change the result's nullability, or make an entirely null input fail during decimal conversion. ### Selecting one field must not hide an explicit cast error For `s = {x: 1, y: 'bad'}`, consider this physical expression: ```sql get_field(CAST(s AS STRUCT<x INT, y INT>), 'x') ``` The explicit cast asks to convert both fields. It must fail because `'bad'` cannot become an integer, even though the caller only uses `x`. Narrowing it to `CAST(get_field(s, 'x') AS INT)` instead returns `1`: the conversion of `y`, and its error, have disappeared. This failure is reproduced directly at the physical-expression adapter boundary. The distinction is whether a cast was inserted to reconcile file and table schemas or was already part of the query; the two can have the same shape but different obligations. ### An entirely null Struct should not acquire a decimal-conversion error Consider a Parquet batch with this schema evolution: ```text File schema: s: Struct<x: Utf8> Table schema: s: Struct<x: Decimal128(10, -1)> Batch values: s is NULL for every row ``` DataFusion's whole-Struct conversion returns nulls without converting its children. If the rewrite extracts `x` first, however, it invokes string-to-decimal conversion on an array of null strings. Arrow rejects the negative scale while setting up that conversion, before looking at any values. A query that should return nulls now fails despite there being no non-null value to convert. The same problem occurs inside a selected container. For example, evolving `s.x` from `List<Utf8>` to `List<Decimal128(10, -1)>` can fail even when every parent Struct is null. Checking only whether `x` itself is a decimal misses the conversion inside the List. This case also reproduces through a Parquet scan with filter pushdown: `WHERE get_field(s, 'x') IS NULL` should select every row in the batch, not fail while preparing a decimal conversion. There is a related schema-contract problem. A required child `x` inside a nullable logical parent `s` still gives a nullable result for `s.x`. Rebuilding an expression from the child's Field alone can lose that inherited nullability, including through nested parents. ## What changes are included in this PR? The adapter now distinguishes casts it introduces for schema adaptation from casts already present in the query. It can continue narrowing generated conversions where that is safe while preserving explicit Struct casts and their errors. Narrowing also preserves the original logical result Field, including metadata and nullability; matching scalar types alone do not establish an equivalent result. Cast tracking also avoids quadratic lookup work for large expressions. For the covered decimal changes, the conversion stays inside its Struct ancestors so DataFusion's existing casting code retains control over the all-null shortcut. The decision looks through container value types too, so selecting a List, Map, or Dictionary does not hide a decimal conversion from the check. The cast target keeps only the selected field path, excluding conversions for unselected siblings. Matching types still narrow normally, and existing container-to-Struct conversion paths are preserved. This extends the existing fix without duplicating Arrow's conversion-validation rules or changing the underlying casting implementation. Unwrapping a Dictionary whose values are Structs retains its existing all-null decimal limitation; the built-in Parquet reader does not produce that source shape. The Parquet reader must then be able to evaluate the retained expression after schema adaptation. This matters when the query plan has already delegated a predicate to the reader: retaining a cast must not prevent that filter from executing. The runtime allowance is specifically for field access through a Struct-to-Struct cast of a column. The reader preserves the cast and reuses projection's existing read clipping to decode only the leaves named by the cast target, with a full-root fallback when clipping is unsafe. The target, rather than the final field selection, determines which conversions must run: an explicit cast that names `y` still reads and converts `y` even if the query selects only `x`. Filter cost estimates use the same selected leaves. Planning remains conservative about explicit casts, keeping a residual filter for them. ## Are these changes tested? The Rust regressions cover explicit cast errors, nullability inherited from parents, decimal conversions on entirely null Structs, unselected sibling conversion errors, and execution through Parquet filters. Container cases include the List families, Maps, Dictionaries, and deeper nesting, with checks that matching types and existing container-to-Struct conversions keep their previous behavior. The existing `schema_evolution_nested.slt` now adds SQL coverage using generated Parquet files: an explicit Struct cast whose unselected sibling must still fail, all-null scalar and List decimal projections, and null predicates with filter pushdown both enabled and disabled. Statistics and pruning shortcuts are disabled for the filter cases. These SQL tests supplement the Rust tests, which also check adapter provenance, logical Field metadata/nullability, expression shape, and Arrow types that are difficult to express in SQL. The explicit-cast SQL case is an integration control; the direct adapter test exercises the cast-provenance regression. The retained-cast read-plan regression now uses three physical siblings. It verifies that an unused sibling is pruned, a sibling named by the cast still raises its conversion error, the projected physical schema is correct, and the estimated compressed bytes count only the selected leaves. A target containing just the selected field also decodes and evaluates successfully. Local validation for [ec4b3f2](apache@ec4b3f2) used the tracked `Cargo.lock` unchanged, with `--locked` for the Cargo test and all-feature Clippy commands: - The required extended workspace suite passed **10,795 Rust tests** (eight existing ignores) and **all 505 SQL logic files**. - All **107 CLI tests** passed. - The adapter and Parquet datasource crates also passed separately: **291 unit tests and eight doctests**, with five existing doctest ignores. - `cargo fmt --all`, full-workspace Clippy across all targets and features with warnings denied, and the complete `./dev/rust_lint.sh` suite passed. The strengthened read-plan regression was also checked with the old full-root collection behavior restored: it failed because it selected leaves `[1, 2, 3]` instead of `[1, 2]`. Restoring the fix made it pass again. <details> <summary>Existing benchmark comparison</summary> The existing benchmarks were compared against `main` at `ee59f628b44eb80e8a4f126288632ef51fda5dc2`, using `release-nonlto`, freshly built workspace artifacts for each revision, and the checked-in Criterion settings. Variants ran sequentially after builds and tests had stopped. | Existing benchmark case | Result | | --- | --- | | `parquet_struct_filter_pushdown`: `select_id/with_pushdown` | No change detected (p = 0.83; 100 samples per variant). | | `parquet_nested_schema_pruning`: `top_level_struct/select_struct_narrow_schema` | PR/main mean-time changes were +4.15%, -2.96%, and -2.84% across three repeats, including reversed execution order (10 samples per variant). The direction changed, so no stable timing conclusion is claimed. | Scan-byte counts were identical in every projection run. These cases cover ordinary field-filter pushdown and the existing projection-clipping path; neither directly measures retained-cast row-filter speedup. The `main` snapshot also contains unrelated commits, so this comparison does not isolate the follow-up patch. </details> ## Are there any user-facing changes? The intended changes preserve explicit-cast errors, logical field metadata and nullability, and the covered all-null decimal behavior. Ordinary field pruning remains enabled. Retained-cast Parquet filters can also avoid reading sibling leaves outside the cast target; required conversions are preserved, with a full-root fallback where necessary. There are no public API or dependency changes. General changes to container-to-Struct conversion semantics, generic `get_field` behavior under null parents, and masking of encoded arrays remain outside this PR's scope. AI assistance: Codex assisted with the implementation, regression tests, PR text, and the stated local checks and source reviews. The row-filter clipping follow-up incorporates Adrian Garcia Badaracco's [recommended change](pydantic@9746f3f), with additional regression coverage. --------- Co-authored-by: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com>

github-actions Bot added sqllogictest SQL Logic Tests (.slt) datasource Changes to the datasource crate labels Aug 5, 2026

adriangb changed the title ~~Claude/datafusion 24109 xkxvqy~~ fix: apply struct field filters when the file schema needs adaptation Aug 5, 2026

adriangb commented Aug 5, 2026

View reviewed changes

github-actions Bot removed the datasource Changes to the datasource crate label Aug 5, 2026

adriangb requested a lite review from Copilot August 5, 2026 20:28

Copilot started reviewing on behalf of adriangb August 5, 2026 20:28 View session

Copilot AI reviewed Aug 5, 2026

View reviewed changes

adriangb force-pushed the claude/datafusion-24109-xkxvqy branch from 7ea589d to f72ff6b Compare August 6, 2026 13:56

github-actions Bot added the datasource Changes to the datasource crate label Aug 6, 2026

zhuqi-lucas reviewed Aug 6, 2026

View reviewed changes

alamb reviewed Aug 6, 2026

View reviewed changes

claude and others added 7 commits August 19, 2026 17:52

adriangb force-pushed the claude/datafusion-24109-xkxvqy branch from 095bb27 to 6081cc1 Compare August 19, 2026 18:13

github-actions Bot removed the datasource Changes to the datasource crate label Aug 19, 2026

zhuqi-lucas approved these changes Aug 20, 2026

View reviewed changes

adriangb added this pull request to the merge queue Aug 20, 2026

Merged via the queue into apache:main with commit 40c208e Aug 20, 2026
40 checks passed

adriangb deleted the claude/datafusion-24109-xkxvqy branch August 20, 2026 11:18

alamb mentioned this pull request Aug 20, 2026

[branch-55] fix: apply struct field filters when the file schema needs adaptation (#24125) #24530

Merged

blinding-pixels mentioned this pull request Aug 23, 2026

Extend Parquet nested schema pruning to filter pushdown #24120

Open

adriangb mentioned this pull request Aug 24, 2026

Make filter pushdown API more precise for their purpose #18856

Closed

This was referenced Aug 25, 2026

Struct-cast narrowing can suppress cast errors and change field semantics #24679

Closed

fix: preserve explicit Struct casts and evolved field semantics #24680

Merged

alamb mentioned this pull request Aug 27, 2026

Release DataFusion 55.1.0 (minor/patch) Release (Sep 2026) #24462

Open

23 tasks

Uh oh!

Conversation

adriangb commented Aug 5, 2026

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Not addressed here

Are these changes tested?

Are there any user-facing changes?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

codecov-commenter commented Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adriangb Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

timsaucer commented Aug 11, 2026

Uh oh!

adriangb commented Aug 11, 2026

Uh oh!

alamb commented Aug 11, 2026

Uh oh!

alamb commented Aug 11, 2026

Uh oh!

zhuqi-lucas left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alamb commented Aug 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

codecov-commenter commented Aug 5, 2026 •

edited

Loading

adriangb Aug 19, 2026 •

edited

Loading

zhuqi-lucas left a comment •

edited

Loading