From 0f2567e1d9f3a12b4176cf3c6b9f62287cf61642 Mon Sep 17 00:00:00 2001 From: Liang-Chi Hsieh Date: Sat, 30 Jul 2022 09:28:09 -0700 Subject: [PATCH] Fix max and min value for decimal256 --- arrow/src/array/builder/decimal_builder.rs | 16 +++++++--------- arrow/src/datatypes/datatype.rs | 16 +++++++++------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/arrow/src/array/builder/decimal_builder.rs b/arrow/src/array/builder/decimal_builder.rs index 09127e76f5a..22c1490e86f 100644 --- a/arrow/src/array/builder/decimal_builder.rs +++ b/arrow/src/array/builder/decimal_builder.rs @@ -261,7 +261,7 @@ mod tests { use crate::array::array_decimal::{BasicDecimalArray, Decimal128Array}; use crate::array::{array_decimal, Array}; use crate::datatypes::DataType; - use crate::util::decimal::Decimal128; + use crate::util::decimal::{Decimal128, Decimal256}; #[test] fn test_decimal_builder() { @@ -357,31 +357,29 @@ mod tests { #[test] #[should_panic( - expected = "9999999999999999999999999999999999999999999999999999999999999999999999999999 is too large to store in a Decimal256 of precision 76. Max is 999999999999999999999999999999999999999999999999999999999999999999999999999" + expected = "9999999999999999999999999999999999999999999999999999999999999999999999999999 is too large to store in a Decimal256 of precision 75. Max is 999999999999999999999999999999999999999999999999999999999999999999999999999" )] fn test_decimal256_builder_out_of_range_precision_scale() { - let mut builder = Decimal256Builder::new(30, 76, 6); + let mut builder = Decimal256Builder::new(30, 75, 6); let big_value = BigInt::from_str_radix("9999999999999999999999999999999999999999999999999999999999999999999999999999", 10).unwrap(); - let bytes = big_value.to_signed_bytes_le(); - let value = Decimal256::try_new_from_bytes(76, 6, &bytes).unwrap(); + let value = Decimal256::from_big_int(&big_value, 75, 6).unwrap(); builder.append_value(&value).unwrap(); } #[test] #[should_panic( - expected = "9999999999999999999999999999999999999999999999999999999999999999999999999999 is too large to store in a Decimal256 of precision 76. Max is 999999999999999999999999999999999999999999999999999999999999999999999999999" + expected = "9999999999999999999999999999999999999999999999999999999999999999999999999999 is too large to store in a Decimal256 of precision 75. Max is 999999999999999999999999999999999999999999999999999999999999999999999999999" )] fn test_decimal256_data_validation() { - let mut builder = Decimal256Builder::new(30, 76, 6); + let mut builder = Decimal256Builder::new(30, 75, 6); // Disable validation at builder unsafe { builder.disable_value_validation(); } let big_value = BigInt::from_str_radix("9999999999999999999999999999999999999999999999999999999999999999999999999999", 10).unwrap(); - let bytes = big_value.to_signed_bytes_le(); - let value = Decimal256::try_new_from_bytes(76, 6, &bytes).unwrap(); + let value = Decimal256::from_big_int(&big_value, 75, 6).unwrap(); builder .append_value(&value) .expect("should not validate invalid value at builder"); diff --git a/arrow/src/datatypes/datatype.rs b/arrow/src/datatypes/datatype.rs index 9f23e6f790a..034920d3753 100644 --- a/arrow/src/datatypes/datatype.rs +++ b/arrow/src/datatypes/datatype.rs @@ -309,7 +309,6 @@ pub const MAX_DECIMAL_FOR_EACH_PRECISION: [i128; 38] = [ /// `MAX_DECIMAL_FOR_LARGER_PRECISION[p]` holds the maximum integer value /// that can be stored in [DataType::Decimal256] value of precision `p` > 38 pub const MAX_DECIMAL_FOR_LARGER_PRECISION: [&str; 38] = [ - "99999999999999999999999999999999999999", "999999999999999999999999999999999999999", "9999999999999999999999999999999999999999", "99999999999999999999999999999999999999999", @@ -347,6 +346,7 @@ pub const MAX_DECIMAL_FOR_LARGER_PRECISION: [&str; 38] = [ "9999999999999999999999999999999999999999999999999999999999999999999999999", "99999999999999999999999999999999999999999999999999999999999999999999999999", "999999999999999999999999999999999999999999999999999999999999999999999999999", + "9999999999999999999999999999999999999999999999999999999999999999999999999999", ]; /// `MIN_DECIMAL_FOR_EACH_PRECISION[p]` holds the minimum `i128` value @@ -395,7 +395,6 @@ pub const MIN_DECIMAL_FOR_EACH_PRECISION: [i128; 38] = [ /// `MIN_DECIMAL_FOR_LARGER_PRECISION[p]` holds the minimum integer value /// that can be stored in a [DataType::Decimal256] value of precision `p` > 38 pub const MIN_DECIMAL_FOR_LARGER_PRECISION: [&str; 38] = [ - "-99999999999999999999999999999999999999", "-999999999999999999999999999999999999999", "-9999999999999999999999999999999999999999", "-99999999999999999999999999999999999999999", @@ -433,6 +432,7 @@ pub const MIN_DECIMAL_FOR_LARGER_PRECISION: [&str; 38] = [ "-9999999999999999999999999999999999999999999999999999999999999999999999999", "-99999999999999999999999999999999999999999999999999999999999999999999999999", "-999999999999999999999999999999999999999999999999999999999999999999999999999", + "-9999999999999999999999999999999999999999999999999999999999999999999999999999", ]; /// The maximum precision for [DataType::Decimal128] values @@ -454,9 +454,11 @@ pub const DECIMAL_DEFAULT_SCALE: usize = 10; /// interpreted as a Decimal number with precision `precision` #[inline] pub(crate) fn validate_decimal_precision(value: i128, precision: usize) -> Result { - // TODO: add validation logic for precision > 38 - if precision > 38 { - return Ok(value); + if precision > DECIMAL128_MAX_PRECISION { + return Err(ArrowError::InvalidArgumentError(format!( + "Max precision of a Decimal128 is {}, but got {}", + DECIMAL128_MAX_PRECISION, precision, + ))); } let max = MAX_DECIMAL_FOR_EACH_PRECISION[precision - 1]; @@ -464,12 +466,12 @@ pub(crate) fn validate_decimal_precision(value: i128, precision: usize) -> Resul if value > max { Err(ArrowError::InvalidArgumentError(format!( - "{} is too large to store in a Decimal of precision {}. Max is {}", + "{} is too large to store in a Decimal128 of precision {}. Max is {}", value, precision, max ))) } else if value < min { Err(ArrowError::InvalidArgumentError(format!( - "{} is too small to store in a Decimal of precision {}. Min is {}", + "{} is too small to store in a Decimal128 of precision {}. Min is {}", value, precision, min ))) } else {