cakeni commented on code in PR #10509:
URL: https://github.com/apache/arrow-rs/pull/10509#discussion_r3848167848
##########
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:
I think this breaks single_decimal_to_float_lossy for scales outside the i8
range. For example, scale = 128 gets cast to -128 here, so x = 1 is formatted
as roughly 1e128 instead of 1e-128. The same narrowing is also used in
decimal_to_f32_rounded_once. Could we use i8::try_from(scale) and fall back to
the arithmetic path when it doesn't fit?
--
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]
