You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "paleolimbot (via GitHub)" <gi...@apache.org> on 2026/05/08 17:15:09 UTC
[I] Propagation of metadata through casts can strips extension type from the destination field and can result in invalid extension types [datafusion]
paleolimbot opened a new issue, #22079:
URL: https://github.com/apache/datafusion/issues/22079
### Describe the bug
After https://github.com/apache/datafusion/pull/21322 and https://github.com/apache/datafusion/pull/21390 the `Cast` "to" field that was added in https://github.com/apache/datafusion/pull/18136 to be able to represent a cast to an extension type (by including field metadata on the cast target) doesn't have its field metadata consistently represented on the output `expr.to_field()`. Instead, the metadata is propagating from the input which often does not make sense (particularly in the intended usage of the cast `field` where the metadata is meaningful)
The main pieces of code that affect the ability of the `Expr::Cast` to represent a cast to an extension type are where the expression field is resolved in the logical expression:
https://github.com/apache/datafusion/blob/53517af674e2540805dab0f688f324537ada3220/datafusion/expr/src/expr_schema.rs#L74-L88
...and where it is resolved in a physical expression:
https://github.com/apache/datafusion/blob/53517af674e2540805dab0f688f324537ada3220/datafusion/physical-expr/src/expressions/cast.rs#L153-L166
An example of where this results in output that is non-sensical/may be rejected by consumers is a cast from a UUID extension type to Utf8 (which will now happily execute but output the arrow.uuid extension name on the Utf8 storage, which is not allowed). Conversely, a cast from Utf8 to UUID *may* work for strings that are 16 characters long (maybe, I forget the casting rules between string and binary), but definitely would not cast to a UUID (before the two noted changes this would have given a clear error.
There are probably some casts that make sense to execute in this way (e.g., a cast where the data type is identical to the input, while usually simplified away, could in theory propagate extension type metadata safely). I am not sure whether it makes sense for other metadata to be propagated in this way (the usage of non-extension metadata is highly variable). I personally think it is safer to strip metadata in a cast (for the same reason that we strip metadata when executing a scalar function).
### To Reproduce
Cast a UUID column to a string and look at the field metadata / try to consume with pyarrow.
### Expected behavior
I would have expected a cast to UUID to error (because the result is not a UUID).
### Additional context
In https://github.com/apache/datafusion/pull/21071 I'm close to a mechanism to actually execute casts to and from extension types, where the output field of a cast to UUID really is a UUID. I can also isolate a smaller change from that PR to fix this behaviour.
cc @adriangb since we discussed this behaviour in one of the above PRs and in the extension types work.
--
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.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: [I] Propagation of metadata through casts can strips extension type from the destination field and can result in invalid extension types [datafusion]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on issue #22079:
URL: https://github.com/apache/datafusion/issues/22079#issuecomment-4431486725
Possibly the core issue is that we don't separate types and metadata (I wish we had/this was possible in arrow-rs!) so we had to put a FieldRef everywhere. Everywhere in DataFusion that I know about, that FieldRef-that-maybe-should-actually-have-been-a-data-type-of-some-kind's metadata is propagated *except* for the cast.
How about:
- `Expr::to_field()`reflects the `Field` metadata specified in the `Cast`
- Whoever creates the `Expr::Cast` can propagate metadata when creating the cast if they feel it's safe to do so
- The `CastExtension` I have prototyped in https://github.com/apache/datafusion/pull/21071 can handle this today (one can just add a cast extension that handles propagating specific metadata that a system knows about.
> Would a good compromise be that arrow extension type metadata specifically is wiped and comes only from the target field but other arbitrary metadata is preserved?
That would unblock my specific use of casting to a `FieldRef`. I'm also happy to PR this change.
> I think part of the problem is that metadata can serve many purposes. Extension types are just one of them.
Can you give some examples where metadata is communicating non-type information that would be useful to propagate and requires a cast and not a scalar function? In the PR that addresses this I can put them in a comment for future readers. The uses of arrow field metadata I know about are all basically trying to communicate type information or statistics, both of which can be fishy through a cast.
Part of this is coming from quite a lot of previous work with GeoParquet, where we tried to communicate type information in Parquet metadata that was aggressively propagated via Arrow schema metadata and rather easily resulted in metadata that was capable of causing silently incorrect results.
> But I donโt think it would be correct either for select col::text to strip arbitrary metadata
Whether it's correct or not, it is behaviour that has existed in 53 versions of DataFusion ๐
--
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: [I] Propagation of metadata through casts can strips extension type from the destination field and can result in invalid extension types [datafusion]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on issue #22079:
URL: https://github.com/apache/datafusion/issues/22079#issuecomment-4454223068
I'm partway through working on this...the "issue" is that there is a different answer for metadata and nullability depending on what you ask:
- `Expr::Cast(Cast { field, ...}) => field.metadata()` is the target cast field. When I added this I intended for its nullability to be ignored and its metadata to be authoritative, but this was modified shortly afterward (I think before DataFusion 53 was released).
- `logical_expr.to_field()` currently returns the source field's nullability and metadata (i.e., everything about the logical target field is ignored except its data type). This is the part causing me problems because enclosing expressions can't detect that something is the right (extension) type (and this happens before I can run an optimizer rule to fix it).
- `physical_expr.return_field()` currently returns `Expr::Cast(Cast { field, ...}) => field` (i.e., ignores any computation done in `to_field()`). The answer is different if `is_default_target_field()` (which most casts are, because they only modify data type and are created via `Cast::new()` which defines the field in this form)...this is the version that propagates metadata and nullability from the source. This is the version that agrees with `logical_expr.to_field()`.
- `physical_expr.nullable()` currently returns the wider of the source + destination nullability. In practice this frequently disagrees with `return_field().is_nullable()` but probably few enough places call it that it doesn't matter.
Touching return field casting causes a lot of downstream failures but I will try to work through what should happen here.
--
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: [I] Propagation of metadata through casts can strips extension type from the destination field and can result in invalid extension types [datafusion]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on issue #22079:
URL: https://github.com/apache/datafusion/issues/22079#issuecomment-4423762788
FWIW the only thing I personally need is for the `Expr::to_field()` to include the extension metadata present in cast `FieldRef`. Everything else I can work around with optimizer rules.
--
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: [I] Propagation of metadata through casts can strips extension type from the destination field and can result in invalid extension types [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on issue #22079:
URL: https://github.com/apache/datafusion/issues/22079#issuecomment-4427439414
I think part of the problem is that metadata can serve many purposes. Extension types are just one of them. In the context of types I agree this interaction with casting is buggy. But I donโt think it would be correct either for `select col::text` to strip arbitrary metadata that might not have anything to do with the type.
Would a good compromise be that arrow extension type metadata specifically is wiped and comes only from the target field but other arbitrary metadata is preserved?
--
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: [I] Propagation of metadata through casts can strips extension type from the destination field and can result in invalid extension types [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on issue #22079:
URL: https://github.com/apache/datafusion/issues/22079#issuecomment-4670384597
Hi @paleolimbot wondering if there's anything I can do to help here?
--
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: [I] Propagation of metadata through casts can strips extension type from the destination field and can result in invalid extension types [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on issue #22079:
URL: https://github.com/apache/datafusion/issues/22079#issuecomment-4431668966
> That would unblock my specific use of casting to a FieldRef. I'm also happy to PR this change.
I think this would be a reasonable change. Cast operators knowing about type metadata makes sense.
> Can you give some examples where metadata is communicating non-type information that would be useful to propagate and requires a cast and not a scalar function?
I'm struggling to think of one atm, which is maybe a sign that the current behavior is wrong. I will see if I can remember what we were doing that https://github.com/apache/datafusion/pull/21390 fixed.
> Whether it's correct or not, it is behaviour that has existed in 53 versions of DataFusion ๐
Related to the point above: https://github.com/apache/datafusion/pull/21390 only made `try_cast` have the same behavior as `cast`. I don't know how back the `cast` behavior goes, but preserving source field metadata was happening before https://github.com/apache/datafusion/pull/21390. So I don't think it's fair to say casts have stripped arbitrary metadata for 53 versions.
--
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: [I] Propagation of metadata through casts can strips extension type from the destination field and can result in invalid extension types [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb closed issue #22079: Propagation of metadata through casts can strips extension type from the destination field and can result in invalid extension types
URL: https://github.com/apache/datafusion/issues/22079
--
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: [I] Propagation of metadata through casts can strips extension type from the destination field and can result in invalid extension types [datafusion]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on issue #22079:
URL: https://github.com/apache/datafusion/issues/22079#issuecomment-4432121737
> So I don't think it's fair to say casts have stripped arbitrary metadata for 53 versions.
That's a great point, I shouldn't have asserted that without checking ๐ฌ . I forget what happened before I updated DataType -> Field in the cast target.
I'll put together a PR for this later today...most of the work is just finding all the places to update and we can hash out what's actually "correct" with some concrete changes.
--
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: [I] Propagation of metadata through casts can strips extension type from the destination field and can result in invalid extension types [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on issue #22079:
URL: https://github.com/apache/datafusion/issues/22079#issuecomment-4454247946
What a headache. Thanks for working through this.
--
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: [I] Propagation of metadata through casts can strips extension type from the destination field and can result in invalid extension types [datafusion]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on issue #22079:
URL: https://github.com/apache/datafusion/issues/22079#issuecomment-4671316688
Sorry, I got hung up because the existing behaviour was tricky to untangle and then forgot about it ๐ฌ . I was also a little bit demoralized from having fixed this once before and not being sure that anybody agrees with me about what should happen here. I'm happy to take another stab this week...I think something like this will work:
- Logical casts always propagate source metadata and nullability, except for the extension name and extension metadata key which are overwritten from the logical cast target field.
- The planner refuses to plan a cast where the target field is an extension type. The other PR can do that once this is sorted (introduces `CastExtension`), or an optimizer rule does just fine for this.
- Physical casts no longer have a target field...replaced with `target: DataType, nullability: NullabilityCast`, where `enum NullabilityCast { PropagateSource, ForceFalse, ForceTrue }`, or something like that.
The issue with the physical cast is that the sentinel field is confusing...two different operations (aligning nullability, performing a computation to align a bare arrow data type) are getting forced on one operator. I'm a little worried that the approach could still end up with a metadata mismatch for the use of the physical cast to align a target schema, and I would still prefer a cast to drop metadata completely (easier to get right, more consistent with scalar functions).
--
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