Minor: Improvements to type coercion rule (#3379)

* Clean up code in type_coercion.rs * fix: improve error message
apache · Sep 6, 2022 · c89b10f · c89b10f
1 parent 08a85db
commit c89b10f
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 33 deletions.
diff --git 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
@@ -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(