[GitHub] [arrow] jorgecarleitao commented on a change in pull request #8340: ARROW-10165: [Rust] [DataFusion]: Remove special case DataFusion casting checks in favor of Arrow cast kernel

[GitBox <gi...@apache.org>]

jorgecarleitao commented on a change in pull request #8340:
URL: https://github.com/apache/arrow/pull/8340#discussion_r500710152



##########
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 unsure on whether we this is advisable:
   
   Doesn't this mean that the plan can fail arbitrarily when a user performs an impossible cast? This can happen like 10hs after the execution starts, when the query finally reaches that point.
   
   I though that the point of having the types available during planning, both logical and physical, was to perform exactly these types of checks. Removing this check seems a regression to me, on which an impossible cast will only be caught during execution.
   
   Isn't it possible to expand `can_coerce_from` to cover the cases that `DataFusion` is missing but that arrow cast kernel supports?




----------------------------------------------------------------
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:
users@infra.apache.org