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 2021/12/13 18:57:09 UTC
[arrow-datafusion] branch master updated: Consolidate decimal error checking (#1438)
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 <an...@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::*;