You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by al...@apache.org on 2022/12/28 12:21:04 UTC

[arrow-datafusion] branch master updated: remove useless logic (#4741)

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

alamb 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 760f10882 remove useless logic (#4741)
760f10882 is described below

commit 760f10882d8c3b1e92ca95d4f9fa6cc050b45000
Author: Kun Liu <li...@apache.org>
AuthorDate: Wed Dec 28 20:20:59 2022 +0800

    remove useless logic (#4741)
---
 datafusion/expr/src/type_coercion/binary.rs    |  1 +
 datafusion/expr/src/type_coercion/functions.rs | 23 -----------------------
 datafusion/physical-expr/src/planner.rs        | 14 +++-----------
 3 files changed, 4 insertions(+), 34 deletions(-)

diff --git a/datafusion/expr/src/type_coercion/binary.rs b/datafusion/expr/src/type_coercion/binary.rs
index 60d214285..6ecb62abd 100644
--- a/datafusion/expr/src/type_coercion/binary.rs
+++ b/datafusion/expr/src/type_coercion/binary.rs
@@ -575,6 +575,7 @@ fn temporal_coercion(lhs_type: &DataType, rhs_type: &DataType) -> Option<DataTyp
         },
         (Timestamp(_, tz), Utf8) => Some(Timestamp(TimeUnit::Nanosecond, tz.clone())),
         (Utf8, Timestamp(_, tz)) => Some(Timestamp(TimeUnit::Nanosecond, tz.clone())),
+        // TODO: need to investigate the result type for the comparison between timestamp and date
         (Timestamp(_, _), Date32) => Some(Date32),
         (Timestamp(_, _), Date64) => Some(Date64),
         (Timestamp(lhs_unit, lhs_tz), Timestamp(rhs_unit, rhs_tz)) => {
diff --git a/datafusion/expr/src/type_coercion/functions.rs b/datafusion/expr/src/type_coercion/functions.rs
index b4a54fdcc..d611b4dd0 100644
--- a/datafusion/expr/src/type_coercion/functions.rs
+++ b/datafusion/expr/src/type_coercion/functions.rs
@@ -177,35 +177,12 @@ pub fn can_coerce_from(type_into: &DataType, type_from: &DataType) -> bool {
         Timestamp(TimeUnit::Nanosecond, None) => {
             matches!(type_from, Null | Timestamp(_, None))
         }
-        Date32 => {
-            matches!(type_from, Null | Timestamp(_, None))
-        }
         Utf8 | LargeUtf8 => true,
         Null => can_cast_types(type_from, type_into),
         _ => false,
     }
 }
 
-/// Returns a common coerced datatype between 2 given datatypes
-///
-/// See the module level documentation for more detail on coercion.
-pub fn get_common_coerced_type(
-    left_datatype: DataType,
-    right_datatype: DataType,
-) -> Result<DataType> {
-    if left_datatype == right_datatype || can_coerce_from(&left_datatype, &right_datatype)
-    {
-        Ok(left_datatype)
-    } else if can_coerce_from(&right_datatype, &left_datatype) {
-        Ok(right_datatype)
-    } else {
-        Err(DataFusionError::Plan(format!(
-            "Datatypes cannot be casted into common type {:?} <-> {:?}",
-            left_datatype, right_datatype
-        )))
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
diff --git a/datafusion/physical-expr/src/planner.rs b/datafusion/physical-expr/src/planner.rs
index 04acf5322..e6b1dedc4 100644
--- a/datafusion/physical-expr/src/planner.rs
+++ b/datafusion/physical-expr/src/planner.rs
@@ -28,7 +28,6 @@ use crate::{
 use arrow::datatypes::{DataType, Schema};
 use datafusion_common::{DFSchema, DataFusionError, Result, ScalarValue};
 use datafusion_expr::expr::Cast;
-use datafusion_expr::type_coercion::functions::get_common_coerced_type;
 use datafusion_expr::{
     binary_expr, Between, BinaryExpr, Expr, GetIndexedField, Like, Operator, TryCast,
 };
@@ -199,16 +198,9 @@ pub fn create_physical_expr(
                     input_schema,
                 )?)),
                 _ => {
-                    let target_datatype = get_common_coerced_type(
-                        lhs.data_type(input_schema)?,
-                        rhs.data_type(input_schema)?,
-                    )?;
-                    binary(
-                        expressions::cast(lhs, input_schema, target_datatype.clone())?,
-                        *op,
-                        expressions::cast(rhs, input_schema, target_datatype)?,
-                        input_schema,
-                    )
+                    // we have coerced both sides into a common type in the logical phase,
+                    // and then perform a binary operation
+                    binary(lhs, *op, rhs, input_schema)
                 }
             }
         }