adriangb opened a new pull request, #24125: URL: https://github.com/apache/datafusion/pull/24125
## Which issue does this PR close? - Closes https://github.com/apache/datafusion/issues/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: ```sql -- 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: 1. `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. 2. 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')`. 3. `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](https://claude.ai/code/session_01MebN5PsVnYvXUeVKju5K7P)_ -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
