You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "adriangb (via GitHub)" <gi...@apache.org> on 2026/09/01 02:55:53 UTC
[PR] feat: give TryCastExpr a target field [datafusion]
adriangb opened a new pull request, #24835:
URL: https://github.com/apache/datafusion/pull/24835
## Which issue does this PR close?
- Part of https://github.com/apache/datafusion/issues/22079
- Part of https://github.com/apache/datafusion/issues/24724
## Stacking
This is **PR 3 of 3** decomposing https://github.com/apache/datafusion/pull/23169.
- Stacked on https://github.com/apache/datafusion/pull/24833, which is itself stacked on https://github.com/apache/datafusion/pull/24831. **Please review those first.**
- The only new commit here is `feat: give TryCastExpr a target field`. Everything below it in the diff belongs to the two PRs underneath.
- PR 2 (https://github.com/apache/datafusion/pull/24834) also branches from #24833; it and this PR are independent of each other.
Opened as a draft while the stack is under review.
## Rationale for this change
`Expr::TryCast` holds a `FieldRef` target so a `TRY_CAST` can name a destination richer than a `DataType` — for example an extension type resolved by a `TypePlanner`, whose `ARROW:extension:name` lives in the field's metadata. The physical `TryCastExpr` stored only a `DataType`, so there was nowhere to put that target, and `create_physical_expr` refused to lower the expression at all:
```
SELECT TRY_CAST(raw AS UUID) FROM ...;
Error during planning: TryCast from FixedSizeBinary(16) to FixedSizeBinary(16)<{"ARROW:extension:name": "arrow.uuid"}> is not supported
```
That is odd on its face: the same query written with `CAST` has worked since https://github.com/apache/datafusion/pull/20836, which gave `CastExpr` a target field. The guard in the planner was the symptom; the missing field was the cause.
## What changes are included in this PR?
`TryCastExpr` gains a `target_field`, mirroring `CastExpr`:
- `TryCastExpr::new_with_target_field(expr, target_field)` is the new field-aware constructor. `TryCastExpr::new(expr, cast_type)` keeps working unchanged and synthesizes a type-only target, so this is purely additive.
- `cast_type()` now reads through the target field; `target_field()` exposes it.
- `try_cast_with_target_field(expr, input_schema, target_field)` is the field-aware builder, exported alongside the existing `try_cast`. It elides the cast only when the cast would be a genuine no-op, exactly as `cast_with_target_field` does — a same-type `TRY_CAST` is still meaningful when it drops metadata.
- `create_physical_expr` passes the logical target field straight through, and the planner guard is deleted.
- `return_field` derives its result from the shared `cast_output_field`, so `TRY_CAST` and `CAST` report their output field by the same rule.
### Proto
`datafusion/proto` does serialize both cast expressions, and both `PhysicalCastNode` and `PhysicalTryCastNode` carried only an `ArrowType`. A cast to an extension type therefore came back from serialization as a plain cast to the storage type, silently losing `ARROW:extension:name`. For `CastExpr` that is a pre-existing gap, present since it gained a target field; for `TryCastExpr` it would be a gap this PR introduces. Fixing only one of the two would leave a confusing asymmetry, so both messages gain the same optional field:
```protobuf
optional datafusion_common.Field target_field = 3;
```
It is written only when the target says more than a data type, so plans that do not use one encode byte for byte as they did before, and a node without it still decodes by falling back to `arrow_type`. Generated code was refreshed with the repository's own `datafusion/proto-models/regen.sh`.
## What is the testing strategy for this PR?
Full `sqllogictest` suite green (504/504 files); `cargo test -p datafusion-expr -p datafusion-expr-common -p datafusion-physical-expr -p datafusion-physical-plan -p datafusion-sql -p datafusion-proto -p datafusion-proto-models -p datafusion-optimizer -p datafusion-substrait --lib --tests` green; `./ci/scripts/rust_clippy.sh` exits 0.
New tests:
- `cast_extension_type_metadata.slt`: `TRY_CAST(... AS UUID)` on a literal and on a column now returns `arrow.uuid` instead of failing to plan. These replace the `statement error` that pinned the old planner guard, and are the cases #23169's reference test file covers at its lines 49 and 66. A third case checks that a `TRY_CAST` naming only a data type still drops the source's metadata.
- `try_cast.rs`: `try_cast_with_target_field_carries_target_metadata`, `same_type_try_cast_is_only_elided_when_it_is_a_no_op`, `target_field_survives_a_proto_round_trip`, `a_type_only_target_field_is_not_encoded`.
- `cast.rs`: `target_field_survives_a_proto_round_trip`, `a_type_only_target_field_is_not_encoded`.
Load-bearing checks:
- routing only the data type through `create_physical_expr` instead of the target field (leaving the guard removed) fails the first new slt case at `cast_extension_type_metadata.slt:51` with `NULL` in place of `arrow.uuid`. Against `main` all three slt cases fail outright, with the planning error above.
- forcing `target_field: None` on the encode side fails both `target_field_survives_a_proto_round_trip` tests, while both `a_type_only_target_field_is_not_encoded` tests keep passing — which is what confirms the "only encode an explicit target" condition is doing something rather than the field always being written.
## Are there any user-facing changes?
Yes, and they are all fixes:
- `TRY_CAST(expr AS <extension type>)` plans and executes instead of failing, and reports the target's metadata.
- A `CAST` or `TRY_CAST` to an extension type keeps that extension type across protobuf serialization.
API changes are additive: `TryCastExpr::new_with_target_field`, `TryCastExpr::target_field`, `try_cast_with_target_field`, and the optional `target_field` on `PhysicalCastNode`/`PhysicalTryCastNode`. `TryCastExpr::new` and `try_cast` keep their signatures and behaviour for a type-only target.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: give TryCastExpr a target field [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on code in PR #24835:
URL: https://github.com/apache/datafusion/pull/24835#discussion_r3901089989
##########
datafusion/sqllogictest/test_files/metadata.slt:
##########
@@ -218,33 +236,35 @@ FROM table_with_metadata;
2020-09-08
2020-09-08
-# Regression test: CAST should preserve source field metadata
+# A CAST does not inherit the source field's metadata: the metadata describes
+# one particular storage type, and the cast produces a different one.
+# See https://github.com/apache/datafusion/issues/22079
query DT
SELECT
CAST(ts AS DATE) as casted,
arrow_metadata(CAST(ts AS DATE), 'metadata_key')
FROM table_with_metadata;
----
-2020-09-08 ts non-nullable field
-2020-09-08 ts non-nullable field
-2020-09-08 ts non-nullable field
+2020-09-08 NULL
+2020-09-08 NULL
+2020-09-08 NULL
-# Regression test: CAST preserves metadata on integer column
+# ... and that holds for a widening cast between integer types too
query IT
SELECT
CAST(id AS BIGINT) as casted,
arrow_metadata(CAST(id AS BIGINT), 'metadata_key')
FROM table_with_metadata;
----
-1 the id field
-NULL the id field
-3 the id field
+1 NULL
+NULL NULL
+3 NULL
# Regression test: CAST with single-argument arrow_metadata (returns full map)
Review Comment:
Does this need updating?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: give TryCastExpr a target field [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb closed pull request #24835: feat: give TryCastExpr a target field
URL: https://github.com/apache/datafusion/pull/24835
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: give TryCastExpr a target field [datafusion]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #24835:
URL: https://github.com/apache/datafusion/pull/24835#issuecomment-5490241222
## [Codecov](https://app.codecov.io/gh/apache/datafusion/pull/24835?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `84.81013%` with `84 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 81.58%. Comparing base ([`a274959`](https://app.codecov.io/gh/apache/datafusion/commit/a2749598bea2e65241fdbf011a4aac95b58079a7?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`330214f`](https://app.codecov.io/gh/apache/datafusion/commit/330214f032564df430d46ad7af77f0a35d508db7?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 3 commits behind head on main.
| [Files with missing lines](https://app.codecov.io/gh/apache/datafusion/pull/24835?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [datafusion/proto-models/src/generated/pbjson.rs](https://app.codecov.io/gh/apache/datafusion/pull/24835?src=pr&el=tree&filepath=datafusion%2Fproto-models%2Fsrc%2Fgenerated%2Fpbjson.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9wcm90by1tb2RlbHMvc3JjL2dlbmVyYXRlZC9wYmpzb24ucnM=) | 0.00% | [26 Missing :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/24835?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [datafusion/physical-expr/src/expressions/cast.rs](https://app.codecov.io/gh/apache/datafusion/pull/24835?src=pr&el=tree&filepath=datafusion%2Fphysical-expr%2Fsrc%2Fexpressions%2Fcast.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9waHlzaWNhbC1leHByL3NyYy9leHByZXNzaW9ucy9jYXN0LnJz) | 85.00% | [4 Missing and 14 partials :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/24835?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...tafusion/physical-expr/src/expressions/try\_cast.rs](https://app.codecov.io/gh/apache/datafusion/pull/24835?src=pr&el=tree&filepath=datafusion%2Fphysical-expr%2Fsrc%2Fexpressions%2Ftry_cast.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9waHlzaWNhbC1leHByL3NyYy9leHByZXNzaW9ucy90cnlfY2FzdC5ycw==) | 89.03% | [3 Missing and 14 partials :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/24835?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [datafusion/physical-plan/src/projection.rs](https://app.codecov.io/gh/apache/datafusion/pull/24835?src=pr&el=tree&filepath=datafusion%2Fphysical-plan%2Fsrc%2Fprojection.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9waHlzaWNhbC1wbGFuL3NyYy9wcm9qZWN0aW9uLnJz) | 84.15% | [4 Missing and 12 partials :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/24835?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [datafusion/expr/src/expr\_rewriter/mod.rs](https://app.codecov.io/gh/apache/datafusion/pull/24835?src=pr&el=tree&filepath=datafusion%2Fexpr%2Fsrc%2Fexpr_rewriter%2Fmod.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9leHByL3NyYy9leHByX3Jld3JpdGVyL21vZC5ycw==) | 78.26% | [0 Missing and 5 partials :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/24835?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [datafusion/expr-common/src/casts.rs](https://app.codecov.io/gh/apache/datafusion/pull/24835?src=pr&el=tree&filepath=datafusion%2Fexpr-common%2Fsrc%2Fcasts.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9leHByLWNvbW1vbi9zcmMvY2FzdHMucnM=) | 98.73% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/24835?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [datafusion/expr/src/expr\_schema.rs](https://app.codecov.io/gh/apache/datafusion/pull/24835?src=pr&el=tree&filepath=datafusion%2Fexpr%2Fsrc%2Fexpr_schema.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9leHByL3NyYy9leHByX3NjaGVtYS5ycw==) | 97.95% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/24835?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #24835 +/- ##
========================================
Coverage 81.58% 81.58%
========================================
Files 1123 1123
Lines 406610 407094 +484
Branches 406610 407094 +484
========================================
+ Hits 331719 332118 +399
- Misses 55453 55496 +43
- Partials 19438 19480 +42
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/datafusion/pull/24835?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: give TryCastExpr a target field [datafusion]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #24835:
URL: https://github.com/apache/datafusion/pull/24835#issuecomment-5488278954
<!-- semver-check-comment -->
Thank you for opening this pull request!
Reviewer note: [cargo-semver-checks](https://github.com/obi1kenobi/cargo-semver-checks) reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch).
<details>
<summary>Details</summary>
```
Cloning apache/main
Building datafusion-expr v55.0.0 (current)
Built [ 26.779s] (current)
Parsing datafusion-expr v55.0.0 (current)
Parsed [ 0.068s] (current)
Building datafusion-expr v55.0.0 (baseline)
Built [ 25.205s] (baseline)
Parsing datafusion-expr v55.0.0 (baseline)
Parsed [ 0.069s] (baseline)
Checking datafusion-expr v55.0.0 -> v55.0.0 (no change; assume patch)
Checked [ 1.811s] 223 checks: 223 pass, 31 skip
Summary no semver update required
Finished [ 55.380s] datafusion-expr
Building datafusion-expr-common v55.0.0 (current)
Built [ 17.361s] (current)
Parsing datafusion-expr-common v55.0.0 (current)
Parsed [ 0.017s] (current)
Building datafusion-expr-common v55.0.0 (baseline)
Built [ 17.503s] (baseline)
Parsing datafusion-expr-common v55.0.0 (baseline)
Parsed [ 0.017s] (baseline)
Checking datafusion-expr-common v55.0.0 -> v55.0.0 (no change; assume patch)
Checked [ 0.315s] 223 checks: 223 pass, 31 skip
Summary no semver update required
Finished [ 35.832s] datafusion-expr-common
Building datafusion-physical-expr v55.0.0 (current)
Built [ 25.894s] (current)
Parsing datafusion-physical-expr v55.0.0 (current)
Parsed [ 0.043s] (current)
Building datafusion-physical-expr v55.0.0 (baseline)
Built [ 25.776s] (baseline)
Parsing datafusion-physical-expr v55.0.0 (baseline)
Parsed [ 0.044s] (baseline)
Checking datafusion-physical-expr v55.0.0 -> v55.0.0 (no change; assume patch)
Checked [ 0.490s] 223 checks: 223 pass, 31 skip
Summary no semver update required
Finished [ 53.369s] datafusion-physical-expr
Building datafusion-physical-plan v55.0.0 (current)
Built [ 33.852s] (current)
Parsing datafusion-physical-plan v55.0.0 (current)
Parsed [ 0.134s] (current)
Building datafusion-physical-plan v55.0.0 (baseline)
Built [ 34.240s] (baseline)
Parsing datafusion-physical-plan v55.0.0 (baseline)
Parsed [ 0.137s] (baseline)
Checking datafusion-physical-plan v55.0.0 -> v55.0.0 (no change; assume patch)
Checked [ 0.989s] 223 checks: 223 pass, 31 skip
Summary no semver update required
Finished [ 70.708s] datafusion-physical-plan
Building datafusion-proto-models v55.0.0 (current)
Built [ 22.197s] (current)
Parsing datafusion-proto-models v55.0.0 (current)
Parsed [ 0.118s] (current)
Building datafusion-proto-models v55.0.0 (baseline)
Built [ 22.467s] (baseline)
Parsing datafusion-proto-models v55.0.0 (baseline)
Parsed [ 0.120s] (baseline)
Checking datafusion-proto-models v55.0.0 -> v55.0.0 (no change; assume patch)
Checked [ 2.492s] 223 checks: 222 pass, 1 fail, 0 warn, 31 skip
--- failure constructible_struct_adds_field: struct exhaustively constructible through public API adds field ---
Description:
A pub struct that could be exhaustively constructed with a literal using only public API has a new pub field, breaking existing exhaustive literals.
ref: https://doc.rust-lang.org/reference/expressions/struct-expr.html
impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.50.0/src/lints/constructible_struct_adds_field.ron
Failed in:
field PhysicalCastNode.target_field in /home/runner/work/datafusion/datafusion/datafusion/proto-models/src/generated/prost.rs:1865
field PhysicalCastNode.target_field in /home/runner/work/datafusion/datafusion/datafusion/proto-models/src/generated/prost.rs:1865
field PhysicalTryCastNode.target_field in /home/runner/work/datafusion/datafusion/datafusion/proto-models/src/generated/prost.rs:1855
field PhysicalTryCastNode.target_field in /home/runner/work/datafusion/datafusion/datafusion/proto-models/src/generated/prost.rs:1855
Summary semver requires new major version: 1 major and 0 minor checks failed
Finished [ 48.738s] datafusion-proto-models
Building datafusion-sqllogictest v55.0.0 (current)
Built [ 87.663s] (current)
Parsing datafusion-sqllogictest v55.0.0 (current)
Parsed [ 0.021s] (current)
Building datafusion-sqllogictest v55.0.0 (baseline)
Built [ 88.395s] (baseline)
Parsing datafusion-sqllogictest v55.0.0 (baseline)
Parsed [ 0.021s] (baseline)
Checking datafusion-sqllogictest v55.0.0 -> v55.0.0 (no change; assume patch)
Checked [ 0.123s] 223 checks: 223 pass, 31 skip
Summary no semver update required
Finished [ 178.908s] datafusion-sqllogictest
```
</details>
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org