adriangb opened a new pull request, #24864: URL: https://github.com/apache/datafusion/pull/24864
## Which issue does this PR close? - N/A. This is a draft opened to make the alternative concrete for the discussion deferred in https://github.com/apache/datafusion/pull/23169, not a merge request. ## Rationale for this change https://github.com/apache/datafusion/pull/23169 made cast metadata consistent between the logical and physical layers, and stripped the two extension-type keys so that `CAST(uuid_col AS Utf8)` no longer produces a `Utf8` field claiming to be `arrow.uuid`. It kept the long-standing rule that a cast otherwise merges its source field's metadata into the output. The alternative raised on that PR is that the target's metadata should simply be authoritative: a cast produces the field its target describes. A type-only target describes a field with no metadata, so a plain `CAST(expr AS type)` produces none. That was the behaviour proposed in https://github.com/apache/datafusion/issues/22079 and in https://github.com/apache/datafusion/pull/23169#discussion_r3507474686. Merging leaves the output field dependent on both the target and whatever metadata the source happened to carry, and needs a per-key exception list to stay correct — currently two keys, and any future metadata with type semantics would need adding to it. Stamping needs no exception list, because metadata that describes a type cannot outlive the type it describes. This PR exists so the cost of that change is a diff and a test run rather than an estimate. ## What changes are included in this PR? The rule, in the three places that derive a cast's output field: logical `cast_output_field`, `CastExpr::resolved_target_field`, and `TryCastExpr::return_field`. Each loses its type-only branch and its extension-key removal. Two consequences that are not obvious from the rule itself: 1. **Cast elision.** A same-type cast used to be a metadata no-op, so three places elide it when the types already match. Once the target's metadata is authoritative, a same-type cast is meaningful whenever it clears metadata, so those guards elide only when the source also carries no metadata. #23169 already made this move for extension keys specifically; this widens the same condition. The three are `cast_with_target_field`, `try_cast_with_target_field`, and `ArrowCastFunc::simplify`. 2. **Union coercion.** `coerce_exprs_for_schema` cast each branch to the destination's `DataType`. Under the new rule that cast drops the metadata the union's output schema advertises, and the logical and physical schemas stop agreeing (`Internal error: Physical input schema should be the same as the one converted from logical input schema`). It now coerces to the destination *field*, via a new `coerce_expr_to_field`. This is a real consequence of the rule, and is the only part of the change that is not mechanical. ## What is the testing strategy for this PR? The behaviour change is measured rather than described: **it moves 7 assertions, all in `datafusion/sqllogictest/test_files/metadata.slt`**, regenerated with `--complete` (expected results only; no query text changed). Those 7 are the whole blast radius across the 504-file sqllogictest suite. All 7 were added in https://github.com/apache/datafusion/pull/21390, and that PR only made `try_cast` match `cast`, so half of the surface being changed has existed only since 54. Four unit tests that asserted the merge rule are updated to assert the stamp rule. Verified on this branch with CI's own invocations: - `cargo test --profile ci --workspace --lib --tests --bins --no-fail-fast` with CI's feature set: green - full sqllogictest suite: 504/504 - `./ci/scripts/rust_clippy.sh`: exit 0 - `RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --workspace`: clean ## Are there any user-facing changes? Yes, and it is a breaking behaviour change, which is why this is a draft against 56 rather than a backport. `CAST(expr AS type)` and `TRY_CAST(expr AS type)` no longer carry the source column's field metadata into the result. A cast to an explicit target field is unchanged: it already used the target's metadata. Code that relies on metadata surviving a cast can attach it with an alias, or with an optimizer rule. No public API changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
