scovich commented on code in PR #9689:
URL: https://github.com/apache/arrow-rs/pull/9689#discussion_r3087965547
##########
arrow-cast/src/cast/decimal.rs:
##########
@@ -833,11 +844,11 @@ where
if array.is_null(i) {
value_builder.append_null();
} else {
Review Comment:
Ah, I totally missed the spurious error allocation pitfall 🤦. Glad your
benchmarking uncovered it!
##########
arrow-cast/src/cast/decimal.rs:
##########
@@ -833,11 +844,11 @@ where
if array.is_null(i) {
value_builder.append_null();
} else {
Review Comment:
If you really wanted to unify without the overhead, a helper that returns
`Result<T, D::Native>` should do the trick: `Ok(v)` is a valid value, and
`Err(v)` is the out of gamut value. The value would be super cheap, and safe
path does `.ok()` while unsafe path does `.map(|v|
ArrowError::CastError(...))`.
But probably not worth it, especially given that the checked mul/div _also_
produce `ArrowError` via `?`.
##########
arrow-cast/src/cast/decimal.rs:
##########
@@ -884,26 +890,66 @@ where
if array.is_null(i) {
value_builder.append_null();
} else {
- let v = array.value(i).div_checked(div)?;
-
- let value =
- <T::Native as
NumCast>::from::<D::Native>(v).ok_or_else(|| {
- ArrowError::CastError(format!(
- "value of {:?} is out of range {}",
- v,
- T::DATA_TYPE
- ))
- })?;
-
+ let value = cast_single_decimal_to_integer_result::<D,
T::Native>(
+ array.value(i),
+ div,
+ false,
+ T::DATA_TYPE,
+ )?;
value_builder.append_value(value);
}
}
}
}
}
+
Ok(Arc::new(value_builder.finish()))
}
+/// Casting a given decimal to an integer based on given div and scale.
+/// The value is scaled by multiplying or dividing with the div based on the
scale sign.
+/// Returns `None` if the value is overflow or cannot be represented with the
requested precision.
+#[inline]
+pub fn cast_single_decimal_to_integer_opt<D, T>(
+ value: D::Native,
+ div: D::Native,
+ negative: bool,
+) -> Option<T>
+where
+ T: NumCast + ToPrimitive,
+ D: DecimalType + ArrowPrimitiveType,
+ <D as ArrowPrimitiveType>::Native: ToPrimitive,
+{
+ let v = if negative {
+ value.mul_checked(div).ok()?
+ } else {
+ value.div_checked(div).ok()?
+ };
+ T::from::<D::Native>(v)
+}
+
+#[inline]
+fn cast_single_decimal_to_integer_result<D, T>(
+ value: D::Native,
+ div: D::Native,
+ negative: bool,
+ type_name: DataType,
+) -> Result<T, ArrowError>
+where
+ T: NumCast + ToPrimitive,
+ D: DecimalType + ArrowPrimitiveType,
+ <D as ArrowPrimitiveType>::Native: ToPrimitive,
+{
+ let v = if negative {
+ value.mul_checked(div)?
+ } else {
+ value.div_checked(div)?
+ };
+ T::from::<D::Native>(v).ok_or_else(|| {
Review Comment:
Ah tricky indeed.
--
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]
