Jefffrey commented on code in PR #20945:
URL: https://github.com/apache/datafusion/pull/20945#discussion_r3145038441


##########
datafusion/sqllogictest/test_files/spark/array/array.slt:
##########
@@ -85,3 +85,43 @@ query ?
 SELECT array(arrow_cast(array(1,2), 'LargeList(Int64)'), array(3));
 ----
 [[1, 2], [3]]
+
+query TT

Review Comment:
   These tests already pass on main, so what are they testing here?



##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -236,7 +236,7 @@ pub fn data_types(
         get_valid_types(function_name.as_ref(), type_signature, 
current_types)?;
     if valid_types
         .iter()
-        .any(|data_type| data_type == current_types)
+        .any(|data_type| data_types_match(data_type, current_types))

Review Comment:
   We don't need to change this function, it's deprecated and not used anywhere



##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -757,6 +802,10 @@ fn maybe_data_types(
     for (i, valid_type) in valid_types.iter().enumerate() {
         let current_type = &current_types[i];
 
+        // Keep exact equality here. Some kernels such as `make_array`
+        // require nested field names/order to match exactly at runtime.
+        // Structural-equivalence short-circuiting is handled earlier by
+        // `data_types_match`.

Review Comment:
   I don't quite understand this reasoning and how it doesn't apply to the fix 
applied in `fields_with_udf` directly? As in, `fields_with_udf` first tries to 
see if the input fields already match with the valid types of the UDF and 
returns early if so (and this is where the PR applies a fix for equality). Then 
if it doesn't match exactly it'll try to coerce, which is this code path here. 
So why does coercion have this catch of nested fields needing to match exactly 
at runtime when the exact path doesn't?



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