mbutrovich opened a new pull request, #12062: URL: https://github.com/apache/arrow-rs/pull/12062
# Which issue does this PR close? N/A. # Rationale for this change [`decimal_op`](https://github.com/apache/arrow-rs/blob/8e9eec38ba841eb505c2925c078471e3dcab5559/arrow-arith/src/numeric.rs#L1046-L1111) runs decimal `add`, `sub` and `mul`, including `add_wrapping`, `sub_wrapping` and `mul_wrapping`, through `try_op!` with the checked operations, which return at the first overflow. DataFusion calls `add_wrapping` for `+` ([`binary.rs`](https://github.com/apache/datafusion/blob/32bd81fb226ccf1ae5df67ff340d77baa3e73668/datafusion/physical-expr/src/expressions/binary.rs#L641-L642)), so its default decimal arithmetic runs one row at a time. #12058 does the same change for integers. # What changes are included in this PR? - Decimal `add`, `sub` and `mul` compute the wrapping result and an overflow flag for every slot in one pass, with equal scales and when one side is rescaled. If any slot overflowed, they rerun the existing checked kernel, so errors don't change. - `Decimal32` and `Decimal64` `add` and `sub` vectorize. `Decimal128` and `Decimal256` stay scalar and lose the per-row branch and the operand spills of the error path. `i128` `mul` multiplies in `i64` when both operands fit. - `OverflowingOp`, `overflowing_op` and `collect_overflowing` are the same as in #12058, so whichever PR lands second drops its copy. - Null slots of the result hold the wrapping result instead of zero, as they already do for integer `add_wrapping`. - 5cf1987a9 adds the benchmark cases, so the benchmark bot can compare against it. # Are these changes tested? New tests in `arrow-arith/src/numeric.rs` compare the kernels with the scalar checked operations at the edge values of every decimal type, with equal scales and with either side rescaled, as arrays, as one-row slices, and with a scalar on either side. They also cover `Decimal128` `mul` on both sides of the `i64` bounds. TODO: chart from the bot runs. The overflow cases are expected to be slower, because they pay for the new pass and then the checked kernel. With `fail_on_overflow` or ANSI mode, an overflow fails the query, so that cost is paid at most once per partition. Overflowing values in null slots are the case that can repeat on every batch. The extra pass also costs code size: 89 KB for `decimal_op` and the new helper across the four decimal types, against 28 KB before, in the thin-LTO bench binary on aarch64. Writing into `Vec::spare_capacity_mut` and calling `set_len` instead of `collect` was 1.2x faster for rescaled `Decimal128` `add` and the same elsewhere on Apple M5 Max. This PR keeps `collect` to avoid `unsafe`. Bot runs: TODO links to the six runs. # Are there any significant user-facing changes? No. ## AI Disclosure I wrote this PR with help from Claude but understand and support the core approach, implementation, and test coverage. -- 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]
