From 8a5e87169e05b612ce7e2cb5b9a6c59c5c0fdd34 Mon Sep 17 00:00:00 2001 From: Kirill Zaborsky Date: Tue, 3 Oct 2023 22:09:26 +0300 Subject: [PATCH] Remove redundant is_numeric for DataType It's available in arrow-rs since version 3.0.0 Resolves #1613 --- datafusion/expr/src/type_coercion/binary.rs | 27 ++++++++++--------- datafusion/expr/src/type_coercion/mod.rs | 9 ------- .../optimizer/src/analyzer/type_coercion.rs | 4 +-- 3 files changed, 17 insertions(+), 23 deletions(-) diff --git a/datafusion/expr/src/type_coercion/binary.rs b/datafusion/expr/src/type_coercion/binary.rs index 64f814cd958b..19bb16651da5 100644 --- a/datafusion/expr/src/type_coercion/binary.rs +++ b/datafusion/expr/src/type_coercion/binary.rs @@ -27,7 +27,6 @@ use arrow::datatypes::{ use datafusion_common::Result; use datafusion_common::{plan_err, DataFusionError}; -use crate::type_coercion::is_numeric; use crate::Operator; /// The type signature of an instantiation of binary expression @@ -310,10 +309,10 @@ pub fn comparison_coercion(lhs_type: &DataType, rhs_type: &DataType) -> Option Option { use arrow::datatypes::DataType::*; match (lhs_type, rhs_type) { - (Utf8, _) if is_numeric(rhs_type) => Some(Utf8), - (LargeUtf8, _) if is_numeric(rhs_type) => Some(LargeUtf8), - (_, Utf8) if is_numeric(lhs_type) => Some(Utf8), - (_, LargeUtf8) if is_numeric(lhs_type) => Some(LargeUtf8), + (Utf8, _) if rhs_type.is_numeric() => Some(Utf8), + (LargeUtf8, _) if rhs_type.is_numeric() => Some(LargeUtf8), + (_, Utf8) if lhs_type.is_numeric() => Some(Utf8), + (_, LargeUtf8) if lhs_type.is_numeric() => Some(LargeUtf8), _ => None, } } @@ -365,7 +364,7 @@ fn comparison_binary_numeric_coercion( rhs_type: &DataType, ) -> Option { use arrow::datatypes::DataType::*; - if !is_numeric(lhs_type) || !is_numeric(rhs_type) { + if !lhs_type.is_numeric() || !rhs_type.is_numeric() { return None; }; @@ -563,14 +562,18 @@ fn create_decimal256_type(precision: u8, scale: i8) -> DataType { fn both_numeric_or_null_and_numeric(lhs_type: &DataType, rhs_type: &DataType) -> bool { use arrow::datatypes::DataType::*; match (lhs_type, rhs_type) { - (_, Null) => is_numeric(lhs_type), - (Null, _) => is_numeric(rhs_type), + (_, Null) => lhs_type.is_numeric(), + (Null, _) => rhs_type.is_numeric(), (Dictionary(_, lhs_value_type), Dictionary(_, rhs_value_type)) => { - is_numeric(lhs_value_type) && is_numeric(rhs_value_type) + lhs_value_type.is_numeric() && rhs_value_type.is_numeric() } - (Dictionary(_, value_type), _) => is_numeric(value_type) && is_numeric(rhs_type), - (_, Dictionary(_, value_type)) => is_numeric(lhs_type) && is_numeric(value_type), - _ => is_numeric(lhs_type) && is_numeric(rhs_type), + (Dictionary(_, value_type), _) => { + value_type.is_numeric() && rhs_type.is_numeric() + } + (_, Dictionary(_, value_type)) => { + lhs_type.is_numeric() && value_type.is_numeric() + } + _ => lhs_type.is_numeric() && rhs_type.is_numeric(), } } diff --git a/datafusion/expr/src/type_coercion/mod.rs b/datafusion/expr/src/type_coercion/mod.rs index d72d9c50edd2..86005da3dafa 100644 --- a/datafusion/expr/src/type_coercion/mod.rs +++ b/datafusion/expr/src/type_coercion/mod.rs @@ -58,15 +58,6 @@ pub fn is_null(dt: &DataType) -> bool { *dt == DataType::Null } -/// Determine whether the given data type `dt` represents numeric values. -pub fn is_numeric(dt: &DataType) -> bool { - is_signed_numeric(dt) - || matches!( - dt, - DataType::UInt8 | DataType::UInt16 | DataType::UInt32 | DataType::UInt64 - ) -} - /// Determine whether the given data type `dt` is a `Timestamp`. pub fn is_timestamp(dt: &DataType) -> bool { matches!(dt, DataType::Timestamp(_, _)) diff --git a/datafusion/optimizer/src/analyzer/type_coercion.rs b/datafusion/optimizer/src/analyzer/type_coercion.rs index 5e239f8e9934..3b866173859c 100644 --- a/datafusion/optimizer/src/analyzer/type_coercion.rs +++ b/datafusion/optimizer/src/analyzer/type_coercion.rs @@ -41,7 +41,7 @@ use datafusion_expr::type_coercion::functions::data_types; use datafusion_expr::type_coercion::other::{ get_coerce_type_for_case_expression, get_coerce_type_for_list, }; -use datafusion_expr::type_coercion::{is_datetime, is_numeric, is_utf8_or_large_utf8}; +use datafusion_expr::type_coercion::{is_datetime, is_utf8_or_large_utf8}; use datafusion_expr::{ is_false, is_not_false, is_not_true, is_not_unknown, is_true, is_unknown, type_coercion, window_function, AggregateFunction, BuiltinScalarFunction, Expr, @@ -496,7 +496,7 @@ fn coerce_window_frame( let target_type = match window_frame.units { WindowFrameUnits::Range => { if let Some(col_type) = current_types.first() { - if is_numeric(col_type) || is_utf8_or_large_utf8(col_type) { + if col_type.is_numeric() || is_utf8_or_large_utf8(col_type) { col_type } else if is_datetime(col_type) { &DataType::Interval(IntervalUnit::MonthDayNano)