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 4b454d0 Consolidate decimal error checking (#1438) 4b454d0 is described below commit 4b454d0363b034e3ebcdf88f6add2403cd77a23b Author: Andrew Lamb <and...@nerdnetworks.org> AuthorDate: Mon Dec 13 13:54:41 2021 -0500 Consolidate decimal error checking (#1438) --- datafusion/src/sql/planner.rs | 43 +++---------------------------------------- datafusion/src/sql/utils.rs | 29 +++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 40 deletions(-) diff --git a/datafusion/src/sql/planner.rs b/datafusion/src/sql/planner.rs index 72c1962..3558d6c 100644 --- a/datafusion/src/sql/planner.rs +++ b/datafusion/src/sql/planner.rs @@ -36,6 +36,7 @@ use crate::logical_plan::{ use crate::optimizer::utils::exprlist_to_columns; use crate::prelude::JoinType; use crate::scalar::ScalarValue; +use crate::sql::utils::make_decimal_type; use crate::{ error::{DataFusionError, Result}, physical_plan::udaf::AggregateUDF, @@ -372,25 +373,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { Ok(DataType::Utf8) } SQLDataType::Decimal(precision, scale) => { - match (precision, scale) { - (None, _) | (_, None) => { - return Err(DataFusionError::Internal(format!( - "Invalid Decimal type ({:?}), precision or scale can't be empty.", - sql_type - ))); - } - (Some(p), Some(s)) => { - // TODO add bound checker in some utils file or function - if *p > 38 || *s > *p { - return Err(DataFusionError::Internal(format!( - "Error Decimal Type ({:?}), precision must be less than or equal to 38 and scale can't be greater than precision", - sql_type - ))); - } else { - Ok(DataType::Decimal(*p as usize, *s as usize)) - } - } - } + make_decimal_type(*precision, *scale) } SQLDataType::Float(_) => Ok(DataType::Float32), SQLDataType::Real => Ok(DataType::Float32), @@ -2054,27 +2037,7 @@ pub fn convert_data_type(sql_type: &SQLDataType) -> Result<DataType> { SQLDataType::Char(_) | SQLDataType::Varchar(_) => Ok(DataType::Utf8), SQLDataType::Timestamp => Ok(DataType::Timestamp(TimeUnit::Nanosecond, None)), SQLDataType::Date => Ok(DataType::Date32), - SQLDataType::Decimal(precision, scale) => { - match (precision, scale) { - (None, _) | (_, None) => { - return Err(DataFusionError::Internal(format!( - "Invalid Decimal type ({:?}), precision or scale can't be empty.", - sql_type - ))); - } - (Some(p), Some(s)) => { - // TODO add bound checker in some utils file or function - if *p > 38 || *s > *p { - return Err(DataFusionError::Internal(format!( - "Error Decimal Type ({:?})", - sql_type - ))); - } else { - Ok(DataType::Decimal(*p as usize, *s as usize)) - } - } - } - } + SQLDataType::Decimal(precision, scale) => make_decimal_type(*precision, *scale), other => Err(DataFusionError::NotImplemented(format!( "Unsupported SQL type {:?}", other diff --git a/datafusion/src/sql/utils.rs b/datafusion/src/sql/utils.rs index 45e78cb..bce50e5 100644 --- a/datafusion/src/sql/utils.rs +++ b/datafusion/src/sql/utils.rs @@ -17,6 +17,8 @@ //! SQL Utility Functions +use arrow::datatypes::DataType; + use crate::logical_plan::{Expr, LogicalPlan}; use crate::scalar::ScalarValue; use crate::{ @@ -503,6 +505,33 @@ pub(crate) fn group_window_expr_by_sort_keys( Ok(result) } +/// Returns a validated `DataType` for the specified precision and +/// scale +pub(crate) fn make_decimal_type( + precision: Option<u64>, + scale: Option<u64>, +) -> Result<DataType> { + match (precision, scale) { + (None, _) | (_, None) => { + return Err(DataFusionError::Internal(format!( + "Decimal(precision, scale) must both be specified, got ({:?}, {:?})", + precision, scale + ))); + } + (Some(p), Some(s)) => { + // Arrow decimal is i128 meaning 38 maximum decimal digits + if p > 38 || s > p { + return Err(DataFusionError::Internal(format!( + "For decimal(precision, scale) precision must be less than or equal to 38 and scale can't be greater than precision. Got ({}, {})", + p, s + ))); + } else { + Ok(DataType::Decimal(p as usize, s as usize)) + } + } + } +} + #[cfg(test)] mod tests { use super::*;