You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by ag...@apache.org on 2022/09/06 15:52:34 UTC

[arrow-datafusion] branch master updated: Minor: Improvements to type coercion rule (#3379)

This is an automated email from the ASF dual-hosted git repository.

agrove pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow-datafusion.git


The following commit(s) were added to refs/heads/master by this push:
     new c89b10f74 Minor: Improvements to type coercion rule (#3379)
c89b10f74 is described below

commit c89b10f74c9b62f6f482659a7c35a17eb04b48fc
Author: Andrew Lamb <an...@nerdnetworks.org>
AuthorDate: Tue Sep 6 11:52:27 2022 -0400

    Minor: Improvements to type coercion rule (#3379)
    
    * Clean up code in type_coercion.rs
    
    * fix: improve error message
---
 datafusion/optimizer/src/type_coercion.rs | 36 +++++++------------------------
 datafusion/physical-expr/src/planner.rs   | 11 +++++-----
 2 files changed, 14 insertions(+), 33 deletions(-)

diff --git a/datafusion/optimizer/src/type_coercion.rs b/datafusion/optimizer/src/type_coercion.rs
index d9f161599..70f92d4b3 100644
--- a/datafusion/optimizer/src/type_coercion.rs
+++ b/datafusion/optimizer/src/type_coercion.rs
@@ -18,7 +18,6 @@
 //! Optimizer rule for type validation and coercion
 
 use crate::{OptimizerConfig, OptimizerRule};
-use arrow::datatypes::DataType;
 use datafusion_common::{DFSchema, DFSchemaRef, Result};
 use datafusion_expr::binary_rule::coerce_types;
 use datafusion_expr::expr_rewriter::{ExprRewritable, ExprRewriter, RewriteRecursion};
@@ -86,37 +85,18 @@ impl ExprRewriter for TypeCoercionRewriter {
     }
 
     fn mutate(&mut self, expr: Expr) -> Result<Expr> {
-        match &expr {
+        match expr {
             Expr::BinaryExpr { left, op, right } => {
                 let left_type = left.get_type(&self.schema)?;
                 let right_type = right.get_type(&self.schema)?;
-                match right_type {
-                    DataType::Interval(_) => {
-                        // we don't want to cast intervals because that breaks
-                        // the logic in the physical planner
-                        Ok(expr)
-                    }
-                    _ => {
-                        let coerced_type = coerce_types(&left_type, op, &right_type)?;
-                        let left = left.clone().cast_to(&coerced_type, &self.schema)?;
-                        let right = right.clone().cast_to(&coerced_type, &self.schema)?;
-                        match (&left, &right) {
-                            (Expr::Cast { .. }, _) | (_, Expr::Cast { .. }) => {
-                                Ok(Expr::BinaryExpr {
-                                    left: Box::new(left),
-                                    op: *op,
-                                    right: Box::new(right),
-                                })
-                            }
-                            _ => {
-                                // no cast was added so we return the original expression
-                                Ok(expr)
-                            }
-                        }
-                    }
-                }
+                let coerced_type = coerce_types(&left_type, &op, &right_type)?;
+                Ok(Expr::BinaryExpr {
+                    left: Box::new(left.cast_to(&coerced_type, &self.schema)?),
+                    op,
+                    right: Box::new(right.cast_to(&coerced_type, &self.schema)?),
+                })
             }
-            _ => Ok(expr),
+            expr => Ok(expr),
         }
     }
 }
diff --git a/datafusion/physical-expr/src/planner.rs b/datafusion/physical-expr/src/planner.rs
index c344982b3..9a3ad2a8d 100644
--- a/datafusion/physical-expr/src/planner.rs
+++ b/datafusion/physical-expr/src/planner.rs
@@ -48,11 +48,12 @@ pub fn create_physical_expr(
     execution_props: &ExecutionProps,
 ) -> Result<Arc<dyn PhysicalExpr>> {
     if input_schema.fields.len() != input_dfschema.fields().len() {
-        return Err(DataFusionError::Internal(
-            "create_physical_expr passed Arrow schema and DataFusion \
-            schema with different number of fields"
-                .to_string(),
-        ));
+        return Err(DataFusionError::Internal(format!(
+            "create_physical_expr expected same number of fields, got \
+                     got Arrow schema with {}  and DataFusion schema with {}",
+            input_schema.fields.len(),
+            input_dfschema.fields().len()
+        )));
     }
     match e {
         Expr::Alias(expr, ..) => Ok(create_physical_expr(