davidlghellin opened a new pull request, #10509:
URL: https://github.com/apache/arrow-rs/pull/10509
# Which issue does this PR close?
- Closes #10508.
# Rationale for this change
`cast(decimal, Float64)` and `cast(decimal, Float32)` could return a float
that is not the nearest
one to the decimal's exact value — see the issue for the full analysis and
the failing values.
The short version is two independent double roundings.
`single_decimal_to_float_lossy` computed
`f(x) / 10_f64.powi(scale)`, where `f(x)` rounds the unscaled integer once
it exceeds 2^53 and
`10_f64.powi(scale)` is itself 1 ULP off for some scales a `DECIMAL(38,s)`
can declare; dividing two
wrongly rounded operands does not give the correctly rounded quotient.
Separately, the `Float32`
target computed the `f64` result and narrowed it with `as f32`, rounding a
second time — a decimal
just above an `f32` midpoint can collapse onto that midpoint in `f64`, and
round-half-even then sends
it the wrong way, so fixing the `f64` path alone leaves those cases
unchanged.
The expected result is the float nearest the decimal's exact value: what
parsing the decimal's own
text gives, and what Java's `BigDecimal.doubleValue()` / `floatValue()` give.
# What changes are included in this PR?
- `single_decimal_to_float_lossy` keeps the existing arithmetic as a fast
path, but only where both
operands are exact: `-22 <= scale <= 22` and `|unscaled| < 2^53`. A
negative scale multiplies by
`10^-scale` rather than dividing by `10^scale`, since only the former is
an exact power of ten.
Everything outside that range goes through a correctly rounded
decimal-string parse.
- New `single_decimal_to_f32_lossy`, used by the `Float32` target, narrowing
in one step. Same shape,
smaller exact range: `10^k` is exact in an `f32` only up to `k = 10`
(`5^10` is the largest power
of five that fits the 24-bit significand), and the integer bound is 2^24.
- Both are now `#[inline]` rather than `#[inline(always)]`: the bodies are
no longer a single
division, and the string path should not be forced into every call site.
- `Float16` is unchanged and now consumes the corrected `f64`. It is still
not correctly rounded —
`half::f16::from_str` is `f32::from_str(..).map(f16::from_f32)`, so there
is no correctly rounded
decimal → `f16` conversion to compare against. Noted as a remaining
limitation.
# Are these changes tested?
Yes, five tests in `arrow-cast/src/cast/mod.rs`:
- `Float64` and `Float32` against expected constants, each including control
cases that already
passed before this change.
- `Decimal256`, where the `i256 -> f64` step is least likely to leave the
value intact.
- A test that every value lands on the same float as parsing its own text —
that is the definition of
correctly rounded, and it needs no expected constants. It sweeps both
sides of each fast-path
boundary: the 2^53 / 2^24 significand limits and the scale ±22 / ±10
limits on exact powers of ten.
- A test for values carrying more digits than their declared precision,
which must not be truncated.
Also checked out of tree against randomized sweeps of `(unscaled, scale)`
pairs, comparing every
result to an independently built decimal string, for `Decimal128` to both
`f32` and `f64`: 0
mismatches, where the same sweep failed on a large share of the wide cases
before the change.
# Are there any user-facing changes?
Yes. Casts from decimal to `Float32`/`Float64` can now return a different —
correctly rounded — value
for wide decimals, and `single_decimal_to_f32_lossy` is new public API.
# AI disclosure
This change was developed with AI assistance (Claude).
--
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]
