gene-bordegaray commented on code in PR #24670:
URL: https://github.com/apache/datafusion/pull/24670#discussion_r3906054665
##########
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:
oh I saw that #23169 was approved. so we cannot have that check if that is
going to be merged and in the patch as it will fail some of the tests it
introduces. the commit on top solely fixes 1-3. I think that main qualm is that
condition above, if we are keeping the sematics of casting the same for the
patch (since we ae forced) for this to be fixed and pass CI after the merge
then the check cannot be as simple as above.
Maybe we could kee my check and add a todo, ensuring that these semantics
are meant to be reviewed for a longer term solution? Let me knwo your thoughts
--
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]
