Skip to content

Commit

Permalink
Fix max and min value for decimal256 (#2245)
Browse files Browse the repository at this point in the history
  • Loading branch information
viirya authored Jul 31, 2022
1 parent f41fb1c commit 281cd79
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 22 deletions.
6 changes: 3 additions & 3 deletions arrow/src/array/array_decimal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@ mod tests {
let mut result = decimal_builder.append_value(123456);
let mut error = result.unwrap_err();
assert_eq!(
"Invalid argument error: 123456 is too large to store in a Decimal of precision 5. Max is 99999",
"Invalid argument error: 123456 is too large to store in a Decimal128 of precision 5. Max is 99999",
error.to_string()
);

Expand All @@ -558,7 +558,7 @@ mod tests {
result = decimal_builder.append_value(100);
error = result.unwrap_err();
assert_eq!(
"Invalid argument error: 100 is too large to store in a Decimal of precision 2. Max is 99",
"Invalid argument error: 100 is too large to store in a Decimal128 of precision 2. Max is 99",
error.to_string()
);

Expand Down Expand Up @@ -677,7 +677,7 @@ mod tests {

#[test]
#[should_panic(
expected = "-123223423432432 is too small to store in a Decimal of precision 5. Min is -99999"
expected = "-123223423432432 is too small to store in a Decimal128 of precision 5. Min is -99999"
)]
fn test_decimal_array_with_precision_and_scale_out_of_range() {
Decimal128Array::from_iter_values([12345, 456, 7890, -123223423432432])
Expand Down
16 changes: 7 additions & 9 deletions arrow/src/array/builder/decimal_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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");
Expand Down
2 changes: 1 addition & 1 deletion arrow/src/array/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2841,7 +2841,7 @@ mod tests {
let validation_result = array_data.validate_full();
let error = validation_result.unwrap_err();
assert_eq!(
"Invalid argument error: 123456 is too large to store in a Decimal of precision 5. Max is 99999",
"Invalid argument error: 123456 is too large to store in a Decimal128 of precision 5. Max is 99999",
error.to_string()
);
}
Expand Down
4 changes: 2 additions & 2 deletions arrow/src/compute/kernels/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2230,7 +2230,7 @@ mod tests {
let array = Arc::new(input_decimal_array) as ArrayRef;
let result = cast(&array, &DataType::Decimal128(2, 2));
assert!(result.is_err());
assert_eq!("Invalid argument error: 12345600 is too large to store in a Decimal of precision 2. Max is 99",
assert_eq!("Invalid argument error: 12345600 is too large to store in a Decimal128 of precision 2. Max is 99",
result.unwrap_err().to_string());
}

Expand Down Expand Up @@ -2412,7 +2412,7 @@ mod tests {
let array = Arc::new(array) as ArrayRef;
let casted_array = cast(&array, &DataType::Decimal128(3, 1));
assert!(casted_array.is_err());
assert_eq!("Invalid argument error: 1000 is too large to store in a Decimal of precision 3. Max is 999", casted_array.unwrap_err().to_string());
assert_eq!("Invalid argument error: 1000 is too large to store in a Decimal128 of precision 3. Max is 999", casted_array.unwrap_err().to_string());

// test f32 to decimal type
let array = Float32Array::from(vec![
Expand Down
16 changes: 9 additions & 7 deletions arrow/src/datatypes/datatype.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -433,6 +432,7 @@ pub const MIN_DECIMAL_FOR_LARGER_PRECISION: [&str; 38] = [
"-9999999999999999999999999999999999999999999999999999999999999999999999999",
"-99999999999999999999999999999999999999999999999999999999999999999999999999",
"-999999999999999999999999999999999999999999999999999999999999999999999999999",
"-9999999999999999999999999999999999999999999999999999999999999999999999999999",
];

/// The maximum precision for [DataType::Decimal128] values
Expand All @@ -454,22 +454,24 @@ 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<i128> {
// 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];
let min = MIN_DECIMAL_FOR_EACH_PRECISION[precision - 1];

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 {
Expand Down

0 comments on commit 281cd79

Please sign in to comment.