tustvold commented on code in PR #2923:
URL: https://github.com/apache/arrow-rs/pull/2923#discussion_r1007188822
##########
arrow/src/compute/kernels/cast.rs:
##########
@@ -336,10 +356,25 @@ where
// with_precision_and_scale validates the
// value is within range for the output precision
- cast_primitive_to_decimal(array, |v| v.as_() * mul, precision, scale)
+ cast_primitive_to_decimal128(array, |v| v.as_() * mul, precision, scale)
+}
+
+fn cast_integer_to_decimal256<T: ArrowNumericType>(
+ array: &PrimitiveArray<T>,
+ precision: u8,
+ scale: u8,
+) -> Result<ArrayRef>
+where
+ <T as ArrowPrimitiveType>::Native: AsPrimitive<i256>,
+{
+ let mul: i256 = i256::from_i128(10_i128.pow(scale as u32));
+
+ // with_precision_and_scale validates the
+ // value is within range for the output precision
Review Comment:
This comment isn't correct anymore
##########
arrow/src/compute/kernels/cast.rs:
##########
@@ -361,6 +396,28 @@ where
)
}
+fn cast_floating_point_to_decimal256<T: ArrowNumericType>(
+ array: &PrimitiveArray<T>,
+ precision: u8,
+ scale: u8,
+) -> Result<ArrayRef>
+where
+ <T as ArrowPrimitiveType>::Native: AsPrimitive<f64>,
+{
+ let mul = 10_f64.powi(scale as i32);
+
+ cast_primitive_to_decimal256(
+ array,
+ |v| {
+ // with_precision_and_scale validates the
+ // value is within range for the output precision
Review Comment:
Same here
##########
arrow/src/compute/kernels/cast.rs:
##########
@@ -336,10 +356,25 @@ where
// with_precision_and_scale validates the
// value is within range for the output precision
- cast_primitive_to_decimal(array, |v| v.as_() * mul, precision, scale)
+ cast_primitive_to_decimal128(array, |v| v.as_() * mul, precision, scale)
+}
+
+fn cast_integer_to_decimal256<T: ArrowNumericType>(
+ array: &PrimitiveArray<T>,
+ precision: u8,
+ scale: u8,
+) -> Result<ArrayRef>
+where
+ <T as ArrowPrimitiveType>::Native: AsPrimitive<i256>,
+{
+ let mul: i256 = i256::from_i128(10_i128.pow(scale as u32));
Review Comment:
This can overflow
```
(i64::MAX as i128).checked_mul(10_i128.checked_pow(DECIMAL256_MAX_SCALE as
_).unwrap()).unwrap()
```
I think this needs a pow function on i256 and to then compute based on this
##########
arrow/src/compute/kernels/cast.rs:
##########
@@ -336,10 +356,25 @@ where
// with_precision_and_scale validates the
// value is within range for the output precision
- cast_primitive_to_decimal(array, |v| v.as_() * mul, precision, scale)
+ cast_primitive_to_decimal128(array, |v| v.as_() * mul, precision, scale)
+}
+
+fn cast_integer_to_decimal256<T: ArrowNumericType>(
+ array: &PrimitiveArray<T>,
+ precision: u8,
+ scale: u8,
+) -> Result<ArrayRef>
+where
+ <T as ArrowPrimitiveType>::Native: AsPrimitive<i256>,
+{
+ let mul: i256 = i256::from_i128(10_i128.pow(scale as u32));
+
+ // with_precision_and_scale validates the
+ // value is within range for the output precision
+ cast_primitive_to_decimal256(array, |v| v.as_().wrapping_mul(mul),
precision, scale)
Review Comment:
I am aware the decimal128 conversions did this before, but this can silently
overflow. This is inconsistent with how we handle other integer conversions in
cast_numeric_arrays which uses
https://docs.rs/num-traits/latest/num_traits/cast/trait.NumCast.html
--
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]
