From 3b3c46fc80db88bf86f3a5b9b11f144af81ccbb0 Mon Sep 17 00:00:00 2001 From: Liang-Chi Hsieh Date: Sat, 1 Apr 2023 00:11:17 -0700 Subject: [PATCH 1/4] Handle overflow precision when casting from integer to decimal --- arrow-cast/src/cast.rs | 44 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/arrow-cast/src/cast.rs b/arrow-cast/src/cast.rs index 02b87e73114c..4d0eee0b4fc8 100644 --- a/arrow-cast/src/cast.rs +++ b/arrow-cast/src/cast.rs @@ -358,13 +358,29 @@ where let array = if scale < 0 { match cast_options.safe { - true => array.unary_opt::<_, D>(|v| v.as_().div_checked(scale_factor).ok()), - false => array.try_unary::<_, D, _>(|v| v.as_().div_checked(scale_factor))?, + true => array.unary_opt::<_, D>(|v| { + v.as_().div_checked(scale_factor).ok().and_then(|v| { + (D::validate_decimal_precision(v, precision).is_ok()).then_some(v) + }) + }), + false => array.try_unary::<_, D, _>(|v| { + v.as_().div_checked(scale_factor).and_then(|v| { + D::validate_decimal_precision(v, precision).and_then(|_| Ok(v)) + }) + })?, } } else { match cast_options.safe { - true => array.unary_opt::<_, D>(|v| v.as_().mul_checked(scale_factor).ok()), - false => array.try_unary::<_, D, _>(|v| v.as_().mul_checked(scale_factor))?, + true => array.unary_opt::<_, D>(|v| { + v.as_().mul_checked(scale_factor).ok().and_then(|v| { + (D::validate_decimal_precision(v, precision).is_ok()).then_some(v) + }) + }), + false => array.try_unary::<_, D, _>(|v| { + v.as_().mul_checked(scale_factor).and_then(|v| { + D::validate_decimal_precision(v, precision).and_then(|_| Ok(v)) + }) + })?, } }; @@ -8132,4 +8148,24 @@ mod tests { .unwrap(); assert_eq!(1672531200000000000, c.value(0)); } + + #[test] + fn test_cast_numeric_to_decimal128_precision_overflow() { + let array = Int64Array::from(vec![1234567]); + let array = Arc::new(array) as ArrayRef; + let casted_array = cast_with_options( + &array, + &DataType::Decimal128(7, 3), + &CastOptions { safe: true }, + ); + assert!(casted_array.is_ok()); + assert!(casted_array.unwrap().is_null(0)); + + let casted_array = cast_with_options( + &array, + &DataType::Decimal128(7, 3), + &CastOptions { safe: false }, + ); + assert!(casted_array.is_err()); + } } From 1e7fe652a1e700a901cdc43000feac1cac4e61d7 Mon Sep 17 00:00:00 2001 From: Liang-Chi Hsieh Date: Sat, 1 Apr 2023 00:40:26 -0700 Subject: [PATCH 2/4] fix clippy --- arrow-cast/src/cast.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/arrow-cast/src/cast.rs b/arrow-cast/src/cast.rs index 4d0eee0b4fc8..f2b5f6abff45 100644 --- a/arrow-cast/src/cast.rs +++ b/arrow-cast/src/cast.rs @@ -364,9 +364,9 @@ where }) }), false => array.try_unary::<_, D, _>(|v| { - v.as_().div_checked(scale_factor).and_then(|v| { - D::validate_decimal_precision(v, precision).and_then(|_| Ok(v)) - }) + v.as_() + .div_checked(scale_factor) + .and_then(|v| D::validate_decimal_precision(v, precision).map(|_| v)) })?, } } else { @@ -377,9 +377,9 @@ where }) }), false => array.try_unary::<_, D, _>(|v| { - v.as_().mul_checked(scale_factor).and_then(|v| { - D::validate_decimal_precision(v, precision).and_then(|_| Ok(v)) - }) + v.as_() + .mul_checked(scale_factor) + .and_then(|v| D::validate_decimal_precision(v, precision).map(|_| v)) })?, } }; From f9a4d478a717b1f59fabb5d7910e7ec791641ce8 Mon Sep 17 00:00:00 2001 From: Liang-Chi Hsieh Date: Sat, 1 Apr 2023 01:04:21 -0700 Subject: [PATCH 3/4] Update test --- arrow-cast/src/cast.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/arrow-cast/src/cast.rs b/arrow-cast/src/cast.rs index f2b5f6abff45..5f9a4bba8b95 100644 --- a/arrow-cast/src/cast.rs +++ b/arrow-cast/src/cast.rs @@ -4391,8 +4391,7 @@ mod tests { assert!(casted_array.is_ok()); let array = casted_array.unwrap(); let array: &Decimal128Array = array.as_primitive(); - let err = array.validate_decimal_precision(3); - assert_eq!("Invalid argument error: 1000 is too large to store in a Decimal128 of precision 3. Max is 999", err.unwrap_err().to_string()); + assert!(array.is_null(4)); // test i8 to decimal type with overflow the result type // the 100 will be converted to 1000_i128, but it is out of range for max value in the precision 3. @@ -4401,8 +4400,7 @@ mod tests { assert!(casted_array.is_ok()); let array = casted_array.unwrap(); let array: &Decimal128Array = array.as_primitive(); - let err = array.validate_decimal_precision(3); - assert_eq!("Invalid argument error: 1000 is too large to store in a Decimal128 of precision 3. Max is 999", err.unwrap_err().to_string()); + assert!(array.is_null(4)); // test f32 to decimal type let array = Float32Array::from(vec![ @@ -4560,8 +4558,7 @@ mod tests { assert!(casted_array.is_ok()); let array = casted_array.unwrap(); let array: &Decimal256Array = array.as_primitive(); - let err = array.validate_decimal_precision(3); - assert_eq!("Invalid argument error: 1000 is too large to store in a Decimal256 of precision 3. Max is 999", err.unwrap_err().to_string()); + assert!(array.is_null(4)); // test f32 to decimal type let array = Float32Array::from(vec![ From 37f1367700c8504e33770e98fa37c259a1a4bfb6 Mon Sep 17 00:00:00 2001 From: Liang-Chi Hsieh Date: Sat, 1 Apr 2023 09:38:45 -0700 Subject: [PATCH 4/4] Update test --- arrow-cast/src/cast.rs | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/arrow-cast/src/cast.rs b/arrow-cast/src/cast.rs index 5f9a4bba8b95..efc8d9d8f061 100644 --- a/arrow-cast/src/cast.rs +++ b/arrow-cast/src/cast.rs @@ -8158,11 +8158,31 @@ mod tests { assert!(casted_array.is_ok()); assert!(casted_array.unwrap().is_null(0)); - let casted_array = cast_with_options( + let err = cast_with_options( &array, &DataType::Decimal128(7, 3), &CastOptions { safe: false }, ); - assert!(casted_array.is_err()); + assert_eq!("Invalid argument error: 1234567000 is too large to store in a Decimal128 of precision 7. Max is 9999999", err.unwrap_err().to_string()); + } + + #[test] + fn test_cast_numeric_to_decimal256_precision_overflow() { + let array = Int64Array::from(vec![1234567]); + let array = Arc::new(array) as ArrayRef; + let casted_array = cast_with_options( + &array, + &DataType::Decimal256(7, 3), + &CastOptions { safe: true }, + ); + assert!(casted_array.is_ok()); + assert!(casted_array.unwrap().is_null(0)); + + let err = cast_with_options( + &array, + &DataType::Decimal256(7, 3), + &CastOptions { safe: false }, + ); + assert_eq!("Invalid argument error: 1234567000 is too large to store in a Decimal256 of precision 7. Max is 9999999", err.unwrap_err().to_string()); } }