davidlghellin commented on code in PR #10509:
URL: https://github.com/apache/arrow-rs/pull/10509#discussion_r4146947786
##########
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)
Review Comment:
No good reason — it double-rounds, which is what this PR is about. It
divides in `f32` directly now.
That path is only reachable if parsing a decimal we just formatted fails,
which shouldn't happen.
--
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]
