mbutrovich opened a new issue, #12168: URL: https://github.com/apache/arrow-rs/issues/12168
### Is your feature request related to a problem or challenge? Part of #11421. Callers round the quotient of a division in different ways, and each mode is a sign and remainder correction applied to the truncated `(q, r)`. Kernels that use the divisors from #11467 need these corrections, and today each caller writes its own. | Rounding | Correction to `(q, r)` | Callers today | |---|---|---| | Toward zero | None | arrow `div` and `rem`, Comet [decimal to integer casts](https://github.com/apache/datafusion-comet/blob/8d0bb01a5a99924024e07c1722e6eddd9260e167/native/spark-expr/src/conversion_funcs/numeric.rs#L548) | | Floor (`div_floor`) | Subtract 1 from `q` when `r != 0` and the signs of `n` and `d` differ | Comet [`floor`](https://github.com/apache/datafusion-comet/blob/8d0bb01a5a99924024e07c1722e6eddd9260e167/native/spark-expr/src/math_funcs/floor.rs#L84-L86) | | Ceiling (`div_ceil`) | Add 1 to `q` when `r != 0` and the signs match | Comet [`ceil`](https://github.com/apache/datafusion-comet/blob/8d0bb01a5a99924024e07c1722e6eddd9260e167/native/spark-expr/src/math_funcs/ceil.rs#L84-L86) | | Half away from zero (`HALF_UP` in Java's `RoundingMode`) | Move `q` one step away from zero when `\|r\| >= \|d\| - \|r\|` | arrow-cast [`make_downscaler`](https://github.com/apache/arrow-rs/blob/6b34163d51aa59f7cac2edcb74e8bd1ddc6b9120/arrow-cast/src/cast/decimal.rs#L235-L246), arrow-arith [`divide_and_round`](https://github.com/apache/arrow-rs/blob/6b34163d51aa59f7cac2edcb74e8bd1ddc6b9120/arrow-arith/src/arithmetic.rs#L172-L186), Comet `div_round_half_up` and [`round`](https://github.com/apache/datafusion-comet/blob/8d0bb01a5a99924024e07c1722e6eddd9260e167/native/spark-expr/src/math_funcs/round.rs#L218-L231), DataFusion [`round`](https://github.com/apache/datafusion/blob/db83fccb9ac10f843be04daa5826501d2338caf4/datafusion/functions/src/math/round.rs#L671-L680) | | Euclidean remainder (`rem_euclid`) | Add `\|d\|` to `r` when `r < 0` | Comet shuffle [`pmod`](https://github.com/apache/datafusion-comet/blob/8d0bb01a5a99924024e07c1722e6eddd9260e167/native/shuffle/src/comet_partitioning.rs#L123-L129) | ### Describe the solution you'd like Generic helpers on top of `wrapping_div_rem` instead of separate division routines per rounding mode. The half-up test is written as `|r| >= |d| - |r|` because `2 * |r|` can overflow when `|d|` is close to the type's maximum. For a positive divisor, `div_euclid` can be one unsigned high multiply on the one's complement of `n`, from the same [review](https://github.com/apache/arrow-rs/pull/11467#pullrequestreview-5658920404) of #11467 that also suggested the signed forms tried in #11979. Tests compare against `num::integer::div_floor`, `num::integer::div_ceil`, `rem_euclid`, and a half-up reference built on `/` and `%`, at the edge cases of each type (`MIN`, `MAX`, divisors of 1 and -1, remainders at exactly half). Which of these become public API on the divisor types and which stay in the kernels that need them should be settled here, because the stability policy locks in whatever ships. ### Describe alternatives you've considered Keep the correction in each kernel, as `make_downscaler` and `divide_and_round` do today. Half-up is the only mode that arrow-rs itself uses more than once, so the helpers could start with it. ### Additional context None. -- 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]
