From bba39974bbb699e00f74912e4227fa58145603d6 Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Tue, 6 Sep 2022 07:18:42 -0400 Subject: [PATCH 1/2] Clean up code in type_coercion.rs --- datafusion/optimizer/src/type_coercion.rs | 36 +++++------------------ 1 file changed, 8 insertions(+), 28 deletions(-) diff --git a/datafusion/optimizer/src/type_coercion.rs b/datafusion/optimizer/src/type_coercion.rs index d9f16159926f5..70f92d4b3735a 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 { - 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), } } } From c3f9f27769a1854c67e95e418992d73579268554 Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Tue, 6 Sep 2022 07:21:39 -0400 Subject: [PATCH 2/2] fix: improve error message --- datafusion/physical-expr/src/planner.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/datafusion/physical-expr/src/planner.rs b/datafusion/physical-expr/src/planner.rs index c344982b33796..9a3ad2a8d0d9f 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> { 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(