davidlghellin commented on code in PR #10509:
URL: https://github.com/apache/arrow-rs/pull/10509#discussion_r4146977260
##########
arrow-cast/src/cast/mod.rs:
##########
@@ -88,7 +96,102 @@ where
D: DecimalType,
F: Fn(D::Native) -> f64,
{
- f(x) / 10_f64.powi(scale)
+ let unscaled = f(x);
+ // Both operands are exact in this range, so the division rounds once.
+ if (0..=22).contains(&scale) && unscaled.abs() < F64_EXACT_INT_LIMIT {
+ return unscaled / 10_f64.powi(scale);
+ }
+ decimal_to_f64_rounded_once::<D>(x, scale, unscaled)
+}
+
+/// Rounds once for the values the division cannot convert exactly, by parsing
the
+/// decimal's own text. A scale that does not fit the `i8` `format_decimal`
takes
+/// falls back to the division.
+#[cold]
+#[inline(never)]
+fn decimal_to_f64_rounded_once<D: DecimalType>(x: D::Native, scale: i32,
unscaled: f64) -> f64 {
+ i8::try_from(scale)
+ .ok()
+ .and_then(|scale| D::format_decimal(x, u8::MAX,
scale).parse::<f64>().ok())
+ .unwrap_or_else(|| unscaled / 10_f64.powi(scale))
+}
+
+/// As [`decimal_to_f64_rounded_once`], but narrowing to `f32` in one step.
+/// Rounding to `f64` first and then to `f32` rounds twice: a decimal just
above an
+/// `f32` midpoint can collapse onto that midpoint in `f64`, and
round-half-even
+/// then sends it the wrong way.
+#[cold]
+#[inline(never)]
+fn decimal_to_f32_rounded_once<D: DecimalType>(x: D::Native, scale: i32,
unscaled: f64) -> f32 {
+ i8::try_from(scale)
+ .ok()
+ .and_then(|scale| D::format_decimal(x, u8::MAX,
scale).parse::<f32>().ok())
+ .unwrap_or_else(|| decimal_to_f64_rounded_once::<D>(x, scale,
unscaled) as f32)
+}
+
+/// Casts a decimal array to `Float64`, rounding each value once.
+///
+/// The scale is the same for every value, so the divisor is computed once here
+/// rather than inside the loop.
+fn cast_decimal_to_f64<D, F>(
+ array: &dyn Array,
+ as_float: &F,
+ scale: i32,
+) -> Result<ArrayRef, ArrowError>
+where
+ D: DecimalType + ArrowPrimitiveType,
+ F: Fn(D::Native) -> f64,
+{
+ let array = array.as_primitive::<D>();
+ // `10^scale` is only exactly representable in this range. A negative scale
+ // would have to multiply by `10^-scale` to stay exact, which is not worth
a
+ // second loop: it was not correctly rounded before this change either.
+ if !(0..=22).contains(&scale) {
+ let values = array
+ .unary::<_, Float64Type>(|x| decimal_to_f64_rounded_once::<D>(x,
scale, as_float(x)));
+ return Ok(Arc::new(values));
+ }
+ let pow = 10_f64.powi(scale);
+ let values = array.unary::<_, Float64Type>(|x| {
+ let unscaled = as_float(x);
+ if unscaled.abs() < F64_EXACT_INT_LIMIT {
+ unscaled / pow
+ } else {
+ decimal_to_f64_rounded_once::<D>(x, scale, unscaled)
+ }
+ });
+ Ok(Arc::new(values))
+}
+
+/// Casts a decimal array to `Float32`, rounding each value once. Same shape as
+/// [`cast_decimal_to_f64`] with the bounds an `f32` allows: `10^k` is exact
only up
+/// to `k = 10`, since `5^10` is the largest power of five that fits the 24-bit
Review Comment:
Done, both helpers take an `i8` now. The check moved up to
`single_decimal_to_float_lossy`, which is
the only entry point that can be handed a scale no Arrow decimal can have.
--
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]
