Skip to content

Commit

Permalink
Fix max and min value for decimal256
Browse files Browse the repository at this point in the history
  • Loading branch information
viirya committed Jul 30, 2022
1 parent d727618 commit 0f2567e
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 16 deletions.
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
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 0f2567e

Please sign in to comment.