andygrove opened a new pull request, #5082:
URL: https://github.com/apache/datafusion-comet/pull/5082
## Which issue does this PR close?
Closes #5070.
## Rationale for this change
`spark_round`'s integer path returned `Ok(0)` for every row whenever
`10^(-scale)` did not fit in the native integer type
(`native/spark-expr/src/math_funcs/round.rs` lines 66-72 and 79-96 in the
pre-fix code). For `Int64` with `scale == -19` this masked a real overflow:
values with `|x| >= 5e18` round to `sign(x) * 10^19`, which does not fit in a
`long`. Spark throws `ARITHMETIC_OVERFLOW` under ANSI mode and wraps (low-order
64 bits) under legacy mode; Comet silently returned `0` in both modes.
`Int8`/`Int16`/`Int32` were not affected because their maxima are strictly
less than half the overflowing divisor, so `0` is the correct answer at every
overflowing scale.
## What changes are included in this PR?
- In `round_integer_array!` and `round_integer_scalar!`, when `10^(-scale)`
overflows the native integer type but still fits in `i128`, widen the
intermediate rounding computation to `i128` and check the result against the
native type's range: fail under ANSI, truncate under legacy (matches
`BigDecimal.longValue`'s low-order-64-bit semantics).
- When `10^(-scale)` also overflows `i128` (`-scale > 38`), retain the fast
path that returns `0` — every bounded native integer rounds to `0`.
- Add Rust unit tests covering:
- `Int64` array + scale `-19`, ANSI mode → throws `ARITHMETIC_OVERFLOW`.
- `Int64` array + scale `-19`, legacy mode → wraps to the two's-complement
low 64 bits (`±10^19 as i64`), including `i64::MAX`, `i64::MIN`, `±5e18`, and
values below the rounding threshold.
- `Int64` array + scale `-20`, both modes → always `0`.
- `Int64` scalar equivalents for ANSI throw and legacy wrap.
## Are these changes tested?
Yes: six new tests in `native/spark-expr/src/math_funcs/round.rs`, run via
`cargo test -p datafusion-comet-spark-expr --lib math_funcs::round::`.
## Are there any user-facing changes?
Yes. `round(cast(x as long), s)` where `s <= -19` and `|x| >= 5 * 10^(-s-1)`:
- ANSI mode: now throws `ARITHMETIC_OVERFLOW` (previously returned `0`
silently).
- Legacy mode: now returns the wrapped low-64-bit value (previously returned
`0`).
Both match Apache Spark 3.4–4.1.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
