mbutrovich commented on code in PR #12101:
URL: https://github.com/apache/arrow-rs/pull/12101#discussion_r4492576434
##########
arrow-array/src/array/primitive_array.rs:
##########
@@ -2616,6 +2616,45 @@ mod tests {
);
}
+ #[test]
+ fn test_decimal_validate_precision_with_negative_scale() {
+ fn check<T: DecimalType>()
+ where
+ T::Native: From<i32>,
+ {
+ for scale in [i8::MIN, -2, -1, 0, 2] {
+ let array = PrimitiveArray::<T>::from_iter(
+ [Some(-99), Some(0), Some(1), Some(99), None]
+ .into_iter()
+ .map(|v| v.map(T::Native::from)),
+ )
+ .with_precision_and_scale(2, scale)
+ .unwrap();
+ array.validate_decimal_precision(2).unwrap();
+
+ for value in [-100, 100] {
+ let array =
PrimitiveArray::<T>::from_iter_values([T::Native::from(value)])
+ .with_precision_and_scale(3, scale)
+ .unwrap();
+ assert!(array.validate_decimal_precision(2).is_err());
+ }
+ }
+
Review Comment:
Could we assert the error message for a negative-scale value that doesn't
fit? The `is_err()` checks in the loop above also pass without the fix, because
the old guard rejected every negative-scale array before it looked at any
value. Checking the message shows that these arrays now reach the per-value
check, and that its error formats the value with the negative scale (`100` at
scale -2 is `10000`).
```suggestion
let array =
PrimitiveArray::<T>::from_iter_values([T::Native::from(100)])
.with_precision_and_scale(3, -2)
.unwrap();
assert_eq!(
array.validate_decimal_precision(2).unwrap_err().to_string(),
format!(
"Invalid argument error: 10000 is too large to store in
a {} of precision 2. Max is 9900",
T::PREFIX
)
);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
