Re: [PR] fix(arrow-cast): round decimal to float conversions once [arrow-rs]

["davidlghellin (via GitHub)" <gi...@apache.org>]

davidlghellin commented on code in PR #10509:
URL: https://github.com/apache/arrow-rs/pull/10509#discussion_r3990291776


##########
arrow-cast/src/cast/mod.rs:
##########
@@ -88,7 +95,112 @@ 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. `u8::MAX` because `format_decimal` truncates to the
+/// precision it is given, and values are not validated against the precision they
+/// declare.
+#[cold]
+#[inline(never)]
+fn decimal_to_f64_rounded_once<D: DecimalType>(x: D::Native, scale: i32, unscaled: f64) -> f64 {
+    D::format_decimal(x, u8::MAX, scale as i8)

Review Comment:
   Good catch, thanks! Fixed with `i8::try_from(scale)`, falling back to the division when it doesn't
   fit, as you suggested. Added a test covering both sides of the `i8` range.
   



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org