sunchao opened a new pull request, #24680:
URL: https://github.com/apache/datafusion/pull/24680

   ## Why are the changes needed?
   
   ### Which issue does this PR close?
   
   Closes https://github.com/apache/datafusion/issues/24679. Follow-up to 
#24125 and #24530.
   
   ### Rationale for this change
   
   Struct-cast narrowing currently recognizes a cast by shape alone. An 
explicit cast of `{x: 1, y: 'bad'}` to `Struct<x: Int32, y: Int32>` must fail 
on `y`, even if its caller selects only `x`; narrowing it can remove that 
error. Separately, returning only the selected child Field can lose nullability 
inherited from a parent.
   
   Decimal conversions have another boundary: some validate conversion 
parameters before looking at values. Moving such a conversion outside an 
all-null Struct can turn null results into an error. Keeping the Struct cast 
requires a runtime Parquet filter path that can still evaluate the 
already-pushed predicate.
   
   ## What changes were proposed in this PR?
   
   ### What changes are included in this PR?
   
   Track the identity of adapter-generated Struct casts for one expression 
rewrite and narrow only those casts. Retain strong references so an eliminated 
cast's address cannot be reused by another expression. Newly narrowed Struct 
casts are tracked too, preserving chained field-access optimization.
   
   Use the original `get_field` expression's complete logical return Field when 
rebuilding a cast. Elide that cast only when the rebuilt physical expression 
has the same complete Field.
   
   Keep type-changing decimal conversions inside their Struct cast rather than 
duplicating Arrow's setup checks. Same-type decimal and metadata-only accesses 
still narrow. At runtime, the Parquet filter checker accepts a retained 
`get_field(Cast(Column), literal Struct path)` by reading the full root. 
Planning remains conservative about explicit Struct casts, and output 
projection retains its existing cast-clipping path.
   
   ### Are there any user-facing changes?
   
   Explicit cast errors, logical return-field metadata/nullability, and the 
covered all-null decimal cases are preserved. Evolved decimal fields may read 
more of their Struct to preserve semantics. Ordinary scalar and matching-type 
pruning remain enabled. No public API or dependency changes.
   
   Generic `get_field` null-parent propagation and encoded-array masking are 
unchanged and are not claimed fixed by this PR.
   
   ## How was this PR tested?
   
   ### Are these changes tested?
   
   - Before the production change, three new adapter tests and the 
retained-cast read-plan regression failed. The all-null decimal case also 
failed through an actual Parquet scan. They pass with the fix.
   - The explicit-cast SQL integration test is a positive control: it passes on 
the base too, so it is not counted as an additional reproduction.
   - All 40 expression-adapter, 249 Parquet datasource, and 226 Parquet 
integration tests passed locally. This includes existing nested narrowing and 
matching-decimal optimization controls.
   - Formatting and full-workspace, all-target, all-feature Clippy with 
warnings denied passed.
   - Required extended workspace suite: 10,792 Rust tests passed, 8 ignored, 
and all 505 SQL logic files passed.
   - Ran all six existing Struct-filter benchmarks against the pinned main base 
with `release-nonlto` and independent build directories. Initial `SELECT *` 
with pushdown measured 14.7 ms on base versus 20.2 ms on the patch. A longer 
reverse-order check measured 17.5 ms versus 17.1 ms, with no significant 
difference; unchanged no-pushdown controls also shifted between runs. The 
slowdown was not consistently reproduced, and these measurements do not 
establish a speedup or a general no-regression guarantee.
   - Independent source review found no concrete correctness blocker.
   
   Local dependency caveat: some versions in the upstream lockfile were 
unavailable from the configured registry. Local checks used a temporary 
resolution from that registry, retaining Arrow/Parquet 59.2.0. The original 
tracked lockfile was restored before publication; no dependency changes belong 
to this PR. Exact-upstream-lock validation was not completed locally.
   
   AI assistance: Codex generated the implementation, regression tests, and PR 
text, and performed the stated local checks and source review. This does not 
claim a separate human review.
   


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

Reply via email to