jayzhan211 commented on code in PR #10077:
URL:
https://github.com/apache/arrow-datafusion/pull/10077#discussion_r1565012112
##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -435,15 +437,15 @@ fn coerced_from<'a>(
// Note that not all rules in `comparison_coercion` can be reused here.
// For example, all numeric types can be coerced into Utf8 for
comparison,
// but not for function arguments.
- _ => comparison_binary_numeric_coercion(type_into, type_from).and_then(
- |coerced_type| {
+ _ => comparison_binary_numeric_coercion(type_into, type_from)
+ .or_else(|| dictionary_coercion(type_into, type_from, true))
Review Comment:
> "check inner data type with coerced_from"
Similar to the current implementation of Dict
https://github.com/apache/arrow-datafusion/blob/671cef85c550969ab2c86d644968a048cb181c0c/datafusion/expr/src/type_coercion/functions.rs#L316-L321
The above once checks if the inner type in Dict is coercible by the
`coerced_from` function.
But `dictionary_coercion` checks the inner type of Dict with
`comparison_coercion`.
The `coerced_from` and `comparison_coercion` are slightly different.
`comparison_coercion` cares about the scenario in comparison, so **loss is
allowed**. For example, i64 and u64, we return i64, while we get None in
`coerced_from` for casting u64 to i64.
I tried to have one coercion for all but ended up not finding a way to cover
all the cases in https://github.com/apache/arrow-datafusion/issues/8302.
I suggest we don't mix the logic for `coerce_from` and
`comparison_coercion`. It would be nice to avoid
`comparison_binary_numeric_coercion ` too.
--
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]
