akupchinskiy opened a new pull request, #12049:
URL: https://github.com/apache/arrow-rs/pull/12049
# Which issue does this PR close?
- Closes #12048.
# Rationale for this change
Float-to-decimal casts computed `round(10^scale * x)` in `f64`. From 2^53
on, the result is
wrong: `295161.0` at scale 15 becomes `295160.999999999967232`. At scale 15
and above this
affects most values (see the issue).
# What changes are included in this PR?
In `arrow-cast/src/cast/decimal.rs`:
**Fix.**
- Below 2^53 the result is unchanged: the rounded floating-point product.
- From 2^53 on, the shortest decimal representation of the input is scaled
exactly in integer
arithmetic. This is the representation arrow displays the float with, from
`ryu` (already a
dependency).
- An integer below 2^53 is its own shortest representation, so it is scaled
without formatting.
This applies to `float_to_decimal`, `float_to_decimal_opt` and the array
cast. The doc comment of
`float_to_decimal` describes the switch.
**Array cast structure, which also makes the common casts faster.**
- For `precision <= 15`, every valid value is below 10^15 < 2^53, so a
product of 2^53 or more is
a precision overflow either way and no exact path is needed. The rounded
product is compared
with `10^precision - 1` in `f64` and then converted to `i64`. This also
rejects NaN, infinities
and native overflow.
- Otherwise, a first pass handles products below 2^53 and integers. It has
no calls in the
per-value closure: the precision bounds are precomputed, and the `f64` to
`i64` conversion
replaces the `__fixdfti` call of the `f64` to `i128` one.
- The cast is redone with the exact path only when some value needs it.
Strict casts redo it on
any failure, so they report the same error as before for the first failing
row.
Benchmarks: three new cases in `arrow/benches/cast_kernels.rs`, for
`Decimal128(38, 15)` and
`(38, 20)` casts of random doubles and of integer doubles.
# Are these changes tested?
Yes, in `arrow-cast/src/cast/decimal.rs`:
- `test_float_to_decimal_large_products`: the cases from the issue. It also
checks that values
below 2^53 are unchanged (for example, `1.005` at scale 2 is still `100`),
plus tiny, huge and
non-finite inputs.
- `test_float_to_decimal_matches_reference`: 20,000 random doubles (all four
decimal widths,
scales -40 to 79) and 20,000 doubles whose product lies near 2^53. Below
2^53 they are checked
against the old formula. From 2^53 on they are checked against an
independent reference, which
takes the significant-digit count from `core`'s `Display` and rounds the
exact `{:.1100}`
expansion half-to-even to it. One-off runs with 1M values each also passed.
- `test_cast_float_array_to_decimal_matches_scalar` and
`test_low_precision_float_array_boundaries` check that the array cast
matches the scalar
functions:
- safe and strict casts, with nulls, NaN and infinities;
- mixes of large and small products, across precisions and scales;
- non-zero array offsets;
- in strict casts, the same first error.
Benchmarks (`cast_kernels`, `float(32|64) (integers )?to decimal`), Apple M2
Pro, median time per
array:
| case | main | this PR | change |
|---|---|---|---|
| float64 to decimal128(32, 3), safe | 35.0 µs | 15.9 µs | -54% |
| float32 to decimal32(7, 2), safe | 18.7 µs | 11.3 µs | -39% |
| float32 to decimal32(7, 2), strict | 20.3 µs | 9.9 µs | -51% |
| float64 to decimal128(20, 3), strict | 36.1 µs | 14.6 µs | -60% |
| float64 to decimal256(40, -2), safe | 378 µs | 30.6 µs | -92% |
| float64 to decimal64(3, 0), precision overflow | 30.3 µs | 15.2 µs | -50% |
| float64 integers to decimal128(38, 15) (new) | 35.3 µs | 19.9 µs | -44% |
| float64 to decimal128(38, 15) random (new) | 35.0 µs | 428 µs | +1123% |
| float64 to decimal128(38, 20) random (new) | 35.8 µs | 428 µs | +1095% |
The only slower cases are high-scale casts of non-integers, where the
previous results were
wrong (see the issue). There the cost is formatting the shortest digits with
`ryu`. A `ryu` API
that returns the decimal digits and exponent without formatting would remove
most of it.
# Are there any significant user-facing changes?
Yes: casts whose scaled value is 2^53 or more now return the correct
decimal. Results below 2^53
do not change. No public API changes.
--
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]
