jorgecarleitao commented on a change in pull request #8340:
URL: https://github.com/apache/arrow/pull/8340#discussion_r501257096
##########
File path: rust/datafusion/src/logical_plan/mod.rs
##########
@@ -323,21 +322,19 @@ impl Expr {
///
/// # Errors
///
- /// This function errors when it is impossible to cast the expression to
the target [arrow::datatypes::DataType].
+ /// Currently no errors happen at plan time. If it is impossible
Review comment:
I am sorry, I was confused: `can_coerce_from` checks whether we can
perform a lossless cast. This is different from the ability to perform *any*
cast.
Therefore, I think that the logic we may want is:
* the `Expr::Cast` operation should check that we can perform the cast at
all (e.g. a `ListArray` to a `UInt32Array` should not be allowed). This should
match the casts that the kernel supports.
* implicit casts, such as the ones that we perform when we pass arguments to
functions to try to match the signature, should use `can_coerce_from`, as we do
not want to perform lossless implicit casts.
AFAIK casts to and from dictionaries are lossless, and thus they can happen
at any point in the physical planning. Since all arrays support dictionaries, I
guess we only have to error if the implementation is still not available.
What do you think, @alamb ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
