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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
