From ae4ed0cfa8d1cd743ab08a04582b1972e33aacf2 Mon Sep 17 00:00:00 2001 From: jonahgao Date: Mon, 18 Sep 2023 23:39:48 +0800 Subject: [PATCH 1/2] fix: add missing precision overflow checking for `cast_string_to_decimal` --- arrow-cast/src/cast.rs | 49 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 42 insertions(+), 7 deletions(-) diff --git a/arrow-cast/src/cast.rs b/arrow-cast/src/cast.rs index 7b8e6144bb49..a03720f768c2 100644 --- a/arrow-cast/src/cast.rs +++ b/arrow-cast/src/cast.rs @@ -2801,6 +2801,11 @@ where if cast_options.safe { let iter = from.iter().map(|v| { v.and_then(|v| parse_string_to_decimal_native::(v, scale as usize).ok()) + .and_then(|v| { + T::validate_decimal_precision(v, precision) + .is_ok() + .then_some(v) + }) }); // Benefit: // 20% performance improvement @@ -2815,13 +2820,17 @@ where .iter() .map(|v| { v.map(|v| { - parse_string_to_decimal_native::(v, scale as usize).map_err(|_| { - ArrowError::CastError(format!( - "Cannot cast string '{}' to value of {:?} type", - v, - T::DATA_TYPE, - )) - }) + parse_string_to_decimal_native::(v, scale as usize) + .map_err(|_| { + ArrowError::CastError(format!( + "Cannot cast string '{}' to value of {:?} type", + v, + T::DATA_TYPE, + )) + }) + .and_then(|v| { + T::validate_decimal_precision(v, precision).map(|_| v) + }) }) .transpose() }) @@ -8097,6 +8106,32 @@ mod tests { test_cast_string_to_decimal(array); } + #[test] + fn test_cast_string_to_decimal128_precision_overflow() { + let array = StringArray::from(vec!["1000".to_string()]); + let array = Arc::new(array) as ArrayRef; + let casted_array = cast_with_options( + &array, + &DataType::Decimal128(10, 8), + &CastOptions { + safe: true, + format_options: FormatOptions::default(), + }, + ); + assert!(casted_array.is_ok()); + assert!(casted_array.unwrap().is_null(0)); + + let err = cast_with_options( + &array, + &DataType::Decimal128(10, 8), + &CastOptions { + safe: false, + format_options: FormatOptions::default(), + }, + ); + assert_eq!("Invalid argument error: 100000000000 is too large to store in a Decimal128 of precision 10. Max is 9999999999", err.unwrap_err().to_string()); + } + #[test] fn test_cast_invalid_utf8_to_decimal() { let str_array = StringArray::from(vec!["4.4.5", ". 0.123"]); From 5edaa3a190cb2887442a502fcc00a71c0d3e41bf Mon Sep 17 00:00:00 2001 From: jonahgao Date: Sun, 24 Sep 2023 23:37:52 +0800 Subject: [PATCH 2/2] Add test_cast_string_to_decimal256_precision_overflow --- arrow-cast/src/cast.rs | 78 ++++++++++++++++++++++++++++-------------- 1 file changed, 52 insertions(+), 26 deletions(-) diff --git a/arrow-cast/src/cast.rs b/arrow-cast/src/cast.rs index a03720f768c2..e7727565c981 100644 --- a/arrow-cast/src/cast.rs +++ b/arrow-cast/src/cast.rs @@ -8106,32 +8106,6 @@ mod tests { test_cast_string_to_decimal(array); } - #[test] - fn test_cast_string_to_decimal128_precision_overflow() { - let array = StringArray::from(vec!["1000".to_string()]); - let array = Arc::new(array) as ArrayRef; - let casted_array = cast_with_options( - &array, - &DataType::Decimal128(10, 8), - &CastOptions { - safe: true, - format_options: FormatOptions::default(), - }, - ); - assert!(casted_array.is_ok()); - assert!(casted_array.unwrap().is_null(0)); - - let err = cast_with_options( - &array, - &DataType::Decimal128(10, 8), - &CastOptions { - safe: false, - format_options: FormatOptions::default(), - }, - ); - assert_eq!("Invalid argument error: 100000000000 is too large to store in a Decimal128 of precision 10. Max is 9999999999", err.unwrap_err().to_string()); - } - #[test] fn test_cast_invalid_utf8_to_decimal() { let str_array = StringArray::from(vec!["4.4.5", ". 0.123"]); @@ -8187,6 +8161,32 @@ mod tests { ); } + #[test] + fn test_cast_string_to_decimal128_precision_overflow() { + let array = StringArray::from(vec!["1000".to_string()]); + let array = Arc::new(array) as ArrayRef; + let casted_array = cast_with_options( + &array, + &DataType::Decimal128(10, 8), + &CastOptions { + safe: true, + format_options: FormatOptions::default(), + }, + ); + assert!(casted_array.is_ok()); + assert!(casted_array.unwrap().is_null(0)); + + let err = cast_with_options( + &array, + &DataType::Decimal128(10, 8), + &CastOptions { + safe: false, + format_options: FormatOptions::default(), + }, + ); + assert_eq!("Invalid argument error: 100000000000 is too large to store in a Decimal128 of precision 10. Max is 9999999999", err.unwrap_err().to_string()); + } + #[test] fn test_cast_utf8_to_decimal128_overflow() { let overflow_str_array = StringArray::from(vec![ @@ -8244,6 +8244,32 @@ mod tests { assert!(decimal_arr.is_null(6)); } + #[test] + fn test_cast_string_to_decimal256_precision_overflow() { + let array = StringArray::from(vec!["1000".to_string()]); + let array = Arc::new(array) as ArrayRef; + let casted_array = cast_with_options( + &array, + &DataType::Decimal256(10, 8), + &CastOptions { + safe: true, + format_options: FormatOptions::default(), + }, + ); + assert!(casted_array.is_ok()); + assert!(casted_array.unwrap().is_null(0)); + + let err = cast_with_options( + &array, + &DataType::Decimal256(10, 8), + &CastOptions { + safe: false, + format_options: FormatOptions::default(), + }, + ); + assert_eq!("Invalid argument error: 100000000000 is too large to store in a Decimal256 of precision 10. Max is 9999999999", err.unwrap_err().to_string()); + } + #[test] fn test_cast_utf8_to_decimal256_overflow() { let overflow_str_array = StringArray::from(vec![