gene-bordegaray commented on code in PR #24670:
URL: https://github.com/apache/datafusion/pull/24670#discussion_r3905990481


##########
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:
   yes we could and have CI pass, but there will be the undelying bug that the 
other PR is talking about and the check prposed elides. If we are aware of that 
and ok with it then more than happy to just have the top commit and use the 
check:
   ```rust
       let metadata = if target_field.metadata().is_empty() {
           source_field.metadata().clone()
       } else {
           target_field.metadata().clone()
       };
   ```
   
   This is my first participation in a patch release so some guidance would be 
great. Thank you again 🙇 
   
   EDIT: #23169 properly handles this so if we backpoirt this with it should 
fix all cases for now until we discuss long term semantics



-- 
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]

Reply via email to