Jefffrey commented on code in PR #10707:
URL: https://github.com/apache/arrow-rs/pull/10707#discussion_r3794001727
##########
arrow-cast/src/cast/mod.rs:
##########
@@ -372,31 +385,38 @@ where
))
})?;
+ let overflow = |v: T::Native| {
+ ArrowError::CastError(format!(
+ "Cannot cast to {}({precision}, {scale}). Overflowing on {v:?}",
+ D::PREFIX,
+ ))
+ };
+
let array = if scale < 0 {
match cast_options.safe {
true => array.unary_opt::<_, D>(|v| {
- v.as_()
- .div_checked(scale_factor)
- .ok()
+ integer_to_decimal_native::<_, M>(v)
+ .and_then(|v| v.div_checked(scale_factor).ok())
Review Comment:
this made me realize something interesting, which is the scaling could
actually make the value fit into the decimal bounds. right now we check the
bounds before casting to the native decimal type and then doing a checked
scaling, but if `scale < 0` then this could be overly strict on what values are
allowed in
for `scale > 0` it just ends up with us range checking twice, so thats fine
i think
##########
arrow-cast/src/cast/mod.rs:
##########
@@ -348,6 +348,19 @@ pub fn cast(array: &dyn Array, to_type: &DataType) ->
Result<ArrayRef, ArrowErro
cast_with_options(array, to_type, &CastOptions::default())
}
+/// Convert an integer to a decimal native value without wrapping.
+///
+/// `AsPrimitive` / `as` silently truncates when the source is wider than `M`
+/// (for example `5_000_000_000i64 as i32`). All integer sources fit in `i128`,
+/// so go through that and then use [`DecimalCast`] which is range-checked.
+fn integer_to_decimal_native<I, M>(value: I) -> Option<M>
+where
+ I: NumCast,
+ M: DecimalCast,
+{
+ num_cast::<I, i128>(value).and_then(M::from_decimal)
+}
Review Comment:
do we go through i128 because theres no easy way to go directly from the
input integer type to the native decimal type?
e.g. for i64 -> decimal32, on main we do `i64 as i32` then do the scaling,
this PR seems to do `i32::try_from(i64 as i128)` if im understanding it
correctly?
though maybe LLVM optimizes this away, not sure if we should rely on that
--
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]
