adriangb commented on code in PR #24670:
URL: https://github.com/apache/datafusion/pull/24670#discussion_r3905432158
##########
datafusion/sqllogictest/test_files/cast_extension_type_metadata.slt:
##########
@@ -45,5 +45,55 @@ FROM (
----
00010203040506070809000102030506 arrow.uuid
-statement error DataFusion error: Optimizer rule 'simplify_expressions'
failed[\s\S]*TryCast from FixedSizeBinary\(16\) to
FixedSizeBinary\(16\)<\{"ARROW:extension:name": "arrow\.uuid"\}> is not
supported
-SELECT TRY_CAST(arrow_cast(X'00010203040506070809000102030506',
'FixedSizeBinary(16)') AS UUID);
+# TRY_CAST to extension type should also preserve extension metadata
+query ?T
+SELECT
+ TRY_CAST(
+ arrow_cast(X'00010203040506070809000102030506', 'FixedSizeBinary(16)')
+ AS UUID
+ ),
+ arrow_metadata(
+ TRY_CAST(
+ arrow_cast(X'00010203040506070809000102030506',
'FixedSizeBinary(16)')
+ AS UUID
+ ),
+ 'ARROW:extension:name'
+ );
+----
+00010203040506070809000102030506 arrow.uuid
+
+# TRY_CAST to UUID from a subquery
+query ?T
+SELECT
+ TRY_CAST(raw AS UUID),
+ arrow_metadata(TRY_CAST(raw AS UUID), 'ARROW:extension:name')
+FROM (
+ VALUES (
+ arrow_cast(X'00010203040506070809000102030506', 'FixedSizeBinary(16)')
+ )
+) AS uuids(raw);
+----
+00010203040506070809000102030506 arrow.uuid
+
+# arrow_cast from UUID to same underlying type (FixedSizeBinary(16)) strips
+# extension metadata (type-only cast semantics)
+query ?T
+SELECT
+ arrow_cast(uuid_val, 'FixedSizeBinary(16)'),
+ arrow_metadata(arrow_cast(uuid_val, 'FixedSizeBinary(16)'),
'ARROW:extension:name')
+FROM (
+ SELECT CAST(arrow_cast(X'00010203040506070809000102030506',
'FixedSizeBinary(16)') AS UUID) AS uuid_val
+);
+----
+00010203040506070809000102030506 NULL
+
+# arrow_cast to a different type strips extension metadata (type-only cast
semantics)
+query ?T
Review Comment:
> it has the issue that I talk about
https://github.com/apache/datafusion/pull/24670#discussion_r3865163140 which is
hwy I opted into stacking on https://github.com/apache/datafusion/pull/23169 to
handle that case while not allowing another edge case to creep in
The discussion you linked is suggesting that we go with:
> Does target_field have metadata? if yes, then use that metadata, if not,
then use the source_field metadata.
You propose that breaks with: `arrow_cast(uuid_col, 'FixedSizeBinary')`
because the result is `FixedSizeBinary` but with the UUID metadata (invalid).
Under the stricter proposal in
https://github.com/apache/datafusion/pull/23169#issuecomment-5489686098 this
would be resolved: we'd ignore the source field metadata.
I think the larger question is if we back port the behavior change to `55`.
--
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]
