[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #6206: RFC: Distinguish between argument coercion and output type

["alamb (via GitHub)" <gi...@apache.org>]

alamb commented on code in PR #6206:
URL: https://github.com/apache/arrow-datafusion/pull/6206#discussion_r1183877003


##########
datafusion/expr/src/type_coercion/binary.rs:
##########
@@ -106,39 +106,62 @@ pub fn binary_operator_data_type(
     }
 }
 
-/// Coercion rules for all binary operators. Returns the output type
-/// of applying `op` to an argument of `lhs_type` and `rhs_type`.
-///
-/// Returns None if no suitable type can be found.
-///
-/// TODO this function is trying to serve two purposes at once; it
-/// determines the result type of the binary operation and also
-/// determines how the inputs can be coerced but this results in
-/// inconsistencies in some cases (particular around date + interval)
-/// when the input argument types do not match the output argument
-/// types
+/// The result of applying coercion to a binary operator.
+pub struct Coercion {

Review Comment:
   This is the key difference in this PR -- the result of the coercion calculation that splits up the argument types needed from the output type.  The current API (`coerce_types`) intermixes the two, as explained in https://github.com/apache/arrow-datafusion/issues/3419
   
   I am not sure this would cover all coercions, but I think it would cover the binary ones. 
   
   I also noticed that @viirya  has added some non trivial decimal coercion logic that delays coercion that I don't really understand that I haven't fully worked out



-- 
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@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org