bhaskargurram-ai opened a new pull request, #11332:
URL: https://github.com/apache/arrow-rs/pull/11332
# Which issue does this PR close?
- Closes #11308.
# Rationale for this change
Casts and parsing into decimal types always round half away from zero when
they drop digits. Configurable rounding was a stretch goal of #5068. #10850
consolidated string-to-decimal parsing into one code path and settled the
default rounding, and #11308 tracks what was left: letting callers pick a
different rounding mode, such as half to even or truncation toward zero, while
keeping the current default.
The main concern raised in #5068 was not to regress decimal parsing
performance. So the default mode is designed to run exactly the code it runs
today, with no per-value check of the mode (see the performance notes below).
# What changes are included in this PR?
- A new `DecimalRoundingMode` enum in `arrow-cast` (re-exported as
`arrow_cast::DecimalRoundingMode` and `arrow::compute::DecimalRoundingMode`).
It has three variants: `HalfAwayFromZero` (the default, same as today),
`HalfToEven` and `TowardZero`. The enum is `#[non_exhaustive]` so that other
modes (floor, ceiling, ...) can be added later without a breaking change.
- A new `CastOptions::decimal_rounding` field. These cast paths use it:
- **Utf8 / LargeUtf8 / Utf8View -> Decimal32/64/128/256.** The
consolidated parser from #10850 used to look only at the first discarded digit.
For `HalfToEven` it now also records whether any later discarded digit is
non-zero, so it can tell an exact tie (`2.5`) from a value just above one
(`2.50001`). Ties then go to the even last kept digit. The mode is a
compile-time parameter of the parser (a private `DecimalRounding` trait), and
the cast picks the right copy once per array. This keeps the per-value cost of
the default mode the same as before. It also works with exponents and negative
scales.
- **Float16/32/64 -> decimal.** The rounding function (`f64::round`,
`f64::round_ties_even` or `f64::trunc`) is chosen once per array and inlined
into the loop. As before, it is applied to `value * 10^scale` computed in
`f64`, so the result follows the binary value. For example, `1.005_f64` becomes
`1.00` at scale 2 in every mode. This is noted in the field docs.
- **Decimal -> decimal with a smaller scale** (same or different decimal
width). The downscaler rounds a tie to the even quotient for `HalfToEven` and
drops the remainder for `TowardZero`. The rounding of the quotient goes through
the same `DecimalRounding` trait and is chosen once per array, so the default
per-value code is the same as before.
- A new `arrow_cast::parse::parse_decimal_with_rounding` for standalone
parsing. `parse_decimal` keeps its signature and behaviour.
- In every mode, precision is checked after rounding. For example, `99.995`
fits `Decimal(4, 2)` only with `TowardZero`.
Out of scope here, possible follow-ups:
- CSV and JSON reader options. Both readers call `parse_decimal` today. With
`parse_decimal_with_rounding` in place, they would only need a `ReaderBuilder`
option. I left that out to keep this PR to one crate and to settle the API
first.
- `rescale_decimal` (the public single-value helper) and the Variant
conversions still use the default mode.
- Integer -> decimal with a negative scale truncates today (e.g. `Int32 150`
to `Decimal(5, -2)` is `1`), unlike the other paths, which round half away from
zero. I did not change that default here. It could take the mode too if you
want the paths to be consistent.
# Are these changes tested?
Yes.
- `parse.rs`: `test_parse_decimal_with_rounding` covers all three modes for
Decimal128 and Decimal256. Cases include positive and negative ties, a non-zero
digit after the 5, long inputs that span several accumulator chunks, exponents,
negative scales, and inputs with no discarded digits. It also checks that the
default matches `parse_decimal`.
`test_parse_decimal_with_rounding_checks_precision_after_rounding` covers
precision and native-type overflow.
- `cast/mod.rs`: `test_cast_string_to_decimal_rounding` (Utf8, LargeUtf8 and
Utf8View into all four decimal types, safe and unsafe),
`test_cast_float_to_decimal_rounding` (Float16/32/64),
`test_cast_decimal_to_decimal_rounding` (Decimal128/256 into all four decimal
types, on both the infallible and the precision-checked path) and
`test_cast_to_decimal_rounding_checks_precision_after_rounding`.
- The new tests fail if the mode is ignored (always using the default), or
if `HalfToEven` rounds ties up.
- `cargo test -p arrow-cast` passes (406 unit, 4 integration and 13 doc
tests). `cargo clippy -p arrow-cast --all-targets --all-features -- -D
warnings`, `cargo clippy -p parquet-variant-compute --tests -- -D warnings` and
`cargo fmt --all -- --check` are clean.
Performance: in the default mode, every path runs the same per-value logic
as on `main`. The mode is chosen once per array (string parsing, float and
decimal downscaling), and only `HalfToEven` tracks the extra "any later
non-zero digit" flag while parsing. In an earlier revision the downscaler
matched on the mode for every value, and the benchmark below showed that as +7%
on `decimal128 to decimal128 512 with lower scale (infallible)`. With the mode
chosen once per array, that case measures -1.6%.
I measured on a shared, noisy 4-core VM, so please treat the numbers as
indicative. I built the `main` and branch benchmark binaries, ran them
alternately 3 times, and took the best time of each. These are the decimal
cases of `cargo bench -p arrow --features test_utils --bench cast_kernels --
decimal` (times in µs):
| benchmark | main | this PR | change |
|---|---|---|---|
| cast int64 to decimal32(9, 0) 512 | 1.54 | 1.44 | -6.4% |
| cast int64 to decimal32(9, -1) 512 | 2.92 | 2.89 | -0.9% |
| cast decimal128 to decimal128 512 | 8.25 | 7.72 | -6.5% |
| cast decimal128 to decimal128 512 lower precision | 52.88 | 55.98 | +5.9% |
| cast decimal128 to decimal256 512 | 32.29 | 31.18 | -3.4% |
| cast decimal256 to decimal128 512 | 477.72 | 524.93 | +9.9% |
| cast decimal256 to decimal256 512 | 148.25 | 163.77 | +10.5% |
| cast decimal128 to decimal128 512 with same scale | 0.08 | 0.08 | -3.0% |
| cast decimal128 to decimal128 512 with lower scale (infallible) | 67.40 |
66.33 | -1.6% |
| cast decimal256 to decimal256 512 with same scale | 0.08 | 0.08 | -1.7% |
| cast string to decimal128(38, 3) | 146.61 | 140.83 | -3.9% |
| cast string to decimal256(76, 3) | 187.97 | 207.84 | +10.6% |
| cast float64 to decimal128(32, 3) | 73.69 | 78.82 | +7.0% |
| cast invalid float64 to to decimal128(32, 3) | 36.09 | 37.44 | +3.7% |
| cast decimal128 to float64 | 39.50 | 40.64 | +2.9% |
| cast decimal128 to int8 | 91.53 | 95.02 | +3.8% |
| cast decimal128 to int64 | 75.58 | 77.48 | +2.5% |
| cast decimal256 to float64 | 172.09 | 161.79 | -6.0% |
| cast decimal256 to int64 | 221.60 | 233.71 | +5.5% |
The geometric mean is +1.4%. Paths this PR does not touch (decimal to
int/float, int to decimal, same scale) moved between -6% and +6%, which is
roughly the noise on this machine. Three cases were about +7% to +10% in both
of my runs: `string to decimal256`, `float64 to decimal128` and the Decimal256
downscales. Their per-value logic in the default mode is unchanged, so I
suspect code layout or inlining differences, but I could not confirm that here.
`cargo bench -p arrow-cast --bench parse_decimal`, run the same way, had a
geometric mean of -0.2% over its 38 cases. Individual cases ranged from -21%
(`string decimal32 9 digits`) to +22% (`string decimal128 exponent`). I
compared the disassembly of `parse_decimal::<Decimal128Type>` in the two
benchmark binaries: the instruction mix is nearly identical, with about 25 more
instructions out of roughly 700, mostly register moves.
Given the concern in #5068, it would help if a maintainer could run the
benchmark bot on `parse_decimal` and `cast_kernels`. An earlier revision passed
the mode as a runtime argument instead of a type parameter. I am happy to go
back to that if you prefer the simpler code.
# Are there any user-facing changes?
Yes:
- New public API: `DecimalRoundingMode`, `CastOptions::decimal_rounding` and
`parse_decimal_with_rounding`.
- **Breaking change**: `CastOptions` gets a new public field, so code that
builds it with a struct literal listing every field must add
`..Default::default()` or the new field. A `const` `CastOptions` needs
`decimal_rounding: DecimalRoundingMode::HalfAwayFromZero` because
`Default::default()` is not const. This PR updates the struct literals in this
repository that way (most of the diff in `cast/mod.rs` tests and two in
`parquet-variant-compute`).
Note: #11302 (open) also changes `cast_floating_point_to_decimal`. Whichever
PR merges second will need a small rebase. The float change here is only the
choice of rounding function.
--
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]
