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/04/05 23:18:00 UTC
[PR] fix: preserve source field metadata in TryCast expressions [datafusion]
adriangb opened a new pull request, #21390:
URL: https://github.com/apache/datafusion/pull/21390
## Which issue does this PR close?
N/A — discovered while investigating metadata propagation through cast expressions.
## Rationale for this change
`Expr::Cast` preserves the source field's metadata through a dedicated `to_field` handler in `expr_schema.rs`, but `Expr::TryCast` fell through to the default case which creates a `Field::new(...)` without any metadata. This caused source column metadata to be silently dropped when using `TRY_CAST`.
## What changes are included in this PR?
- Added a dedicated `to_field` handler for `Expr::TryCast` in `expr_schema.rs` that preserves source field metadata (matching `Expr::Cast` behavior), while keeping TryCast's always-nullable semantics.
- Added SLT tests in `metadata.slt` verifying metadata preservation through `TRY_CAST` on both timestamp and integer columns.
## Are these changes tested?
Yes — new sqllogictest cases in `metadata.slt` using `arrow_metadata()` to verify metadata is preserved through `TRY_CAST`.
## Are there any user-facing changes?
`TRY_CAST` now preserves source field metadata, consistent with `CAST` behavior.
🤖 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: 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] fix: preserve source field metadata in TryCast expressions [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on PR #21390:
URL: https://github.com/apache/datafusion/pull/21390#issuecomment-4199026392
Thanks @kosiew and @martin-g !
--
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] fix: preserve source field metadata in TryCast expressions [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on code in PR #21390:
URL: https://github.com/apache/datafusion/pull/21390#discussion_r3039990275
##########
datafusion/expr/src/expr_schema.rs:
##########
@@ -574,12 +574,22 @@ impl ExprSchemable for Expr {
id: _,
field: Some(field),
}) => Ok(Arc::clone(field).renamed(&schema_name)),
+ Expr::TryCast(TryCast { expr, field }) => expr
+ .to_field(schema)
+ .map(|(_table_ref, destination_field)| {
+ destination_field
+ .as_ref()
+ .clone()
+ .with_data_type(field.data_type().clone())
+ .with_nullable(true)
+ .with_metadata(destination_field.metadata().clone())
Review Comment:
addressed
--
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] fix: preserve source field metadata in TryCast expressions [datafusion]
Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew commented on code in PR #21390:
URL: https://github.com/apache/datafusion/pull/21390#discussion_r3042695391
##########
datafusion/expr/src/expr_schema.rs:
##########
@@ -574,12 +574,21 @@ impl ExprSchemable for Expr {
id: _,
field: Some(field),
}) => Ok(Arc::clone(field).renamed(&schema_name)),
+ Expr::TryCast(TryCast { expr, field }) => expr
Review Comment:
I noticed that `Expr::Cast` and `Expr::TryCast` now have very similar `to_field()` handling, with the main difference being how nullability is treated.
Do you think it might be worth pulling this into a small shared helper, something like "derive cast output field from child field"? It feels like the kind of logic that could drift again over time if it stays duplicated, especially if we tweak metadata or nullability behavior later.
##########
datafusion/sqllogictest/test_files/metadata.slt:
##########
@@ -347,5 +369,38 @@ select arrow_metadata(id) from table_with_metadata limit 1;
----
{metadata_key: the id field}
+# Regression test: TRY_CAST should preserve source field metadata
Review Comment:
The new keyed `arrow_metadata(..., 'metadata_key')` cases look great and cover the regression nicely 👍
As a small follow up, it might be worth adding one case for `arrow_metadata(TRY_CAST(...))` using the single argument form too. Since that path returns the full map and this file already exercises both forms for plain columns, it could help round out the coverage.
--
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] fix: preserve source field metadata in TryCast expressions [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb merged PR #21390:
URL: https://github.com/apache/datafusion/pull/21390
--
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] fix: preserve source field metadata in TryCast expressions [datafusion]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on code in PR #21390:
URL: https://github.com/apache/datafusion/pull/21390#discussion_r3038159253
##########
datafusion/expr/src/expr_schema.rs:
##########
@@ -574,12 +574,22 @@ impl ExprSchemable for Expr {
id: _,
field: Some(field),
}) => Ok(Arc::clone(field).renamed(&schema_name)),
+ Expr::TryCast(TryCast { expr, field }) => expr
+ .to_field(schema)
+ .map(|(_table_ref, destination_field)| {
+ destination_field
+ .as_ref()
+ .clone()
+ .with_data_type(field.data_type().clone())
+ .with_nullable(true)
+ .with_metadata(destination_field.metadata().clone())
Review Comment:
Technically this is not really needed. The metadata is cloned with Field::clone()
--
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] fix: preserve source field metadata in TryCast expressions [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on code in PR #21390:
URL: https://github.com/apache/datafusion/pull/21390#discussion_r3038179990
##########
datafusion/expr/src/expr_schema.rs:
##########
@@ -574,12 +574,22 @@ impl ExprSchemable for Expr {
id: _,
field: Some(field),
}) => Ok(Arc::clone(field).renamed(&schema_name)),
+ Expr::TryCast(TryCast { expr, field }) => expr
+ .to_field(schema)
+ .map(|(_table_ref, destination_field)| {
+ destination_field
+ .as_ref()
+ .clone()
+ .with_data_type(field.data_type().clone())
+ .with_nullable(true)
+ .with_metadata(destination_field.metadata().clone())
Review Comment:
```suggestion
```
--
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