klion26 commented on code in PR #9689:
URL: https://github.com/apache/arrow-rs/pull/9689#discussion_r3086693991
##########
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:
Did not unify these two functions, because if I unify them with a common
function like
```
fn cast_single_decimal_to_integer<D, T>(...) -> Result<Option<T>,
ArrowError>> {
let v = if negative {
value.mul_checked(div)?
} else {
value.div_checked(div)?
};
OK(T::from::<D::Native>(v))
}
```
Then, in the caller function, I can't the value of `v` above, this make the
error msg in `cast_single_decimal_to_integer_result` wrong.
##########
arrow-cast/src/cast/decimal.rs:
##########
@@ -833,11 +844,11 @@ where
if array.is_null(i) {
value_builder.append_null();
} else {
Review Comment:
Changed back to the if/esle and match pattern, because
1. We need to distinguish the logic in `safe` and `no-safe` path because of
a performance problem, in the last commit, we will construct an arrowerror(will
call `format!`) and drop it in `safe` mode, this have 50%+ performance
regression in benchmark.
2. After step 1, seems there is little gain to union the logic here
--
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]
