mbutrovich opened a new issue, #11454: URL: https://github.com/apache/arrow-rs/issues/11454
### Is your feature request related to a problem or challenge? [`ArrowNativeTypeOp`](https://github.com/apache/arrow-rs/blob/c7f60c612bd16ca5485cc9acddfd216c10ea9765/arrow-array/src/arithmetic.rs#L41) has [`div_*` and `mod_*` methods](https://github.com/apache/arrow-rs/blob/c7f60c612bd16ca5485cc9acddfd216c10ea9765/arrow-array/src/arithmetic.rs#L77-L94) but none that return both results. Generic decimal code that needs the quotient and the remainder calls both, and for `Decimal256` each call runs Knuth's Algorithm D ([`div_rem_knuth`](https://github.com/apache/arrow-rs/blob/c7f60c612bd16ca5485cc9acddfd216c10ea9765/arrow-buffer/src/bigint/div.rs#L83)). This happens in two places: - [`make_downscaler`](https://github.com/apache/arrow-rs/blob/c7f60c612bd16ca5485cc9acddfd216c10ea9765/arrow-cast/src/cast/decimal.rs#L240-L241) in decimal casts. - [`scaled_div`](https://github.com/apache/arrow-rs/blob/c7f60c612bd16ca5485cc9acddfd216c10ea9765/arrow-arith/src/numeric.rs#L985-L986), which decimal division [falls back to](https://github.com/apache/arrow-rs/blob/c7f60c612bd16ca5485cc9acddfd216c10ea9765/arrow-arith/src/numeric.rs#L1143) when scaling the dividend overflows. The sealed divisor trait planned in #11421 would cover `make_downscaler`, whose divisor is the same for the whole array. It would not cover `scaled_div`, whose divisor changes from row to row. @andygrove raised this while reviewing #11425. ### Describe the solution you'd like Add a wrapping and a checked method to `ArrowNativeTypeOp` that return `(quotient, remainder)`, with default implementations that call the existing `div_*` and `mod_*` methods. `i256` would override them with `i256::wrapping_div_rem` and `i256::checked_div_rem` from #11425, and the callers above would switch to them. `ArrowNativeType` is [sealed](https://github.com/apache/arrow-rs/blob/c7f60c612bd16ca5485cc9acddfd216c10ea9765/arrow-buffer/src/native.rs#L53-L54), so no downstream crate implements `ArrowNativeTypeOp` and the new methods are not a breaking change. The existing methods are named `div_*` and `mod_*`, while `i256` uses `div_rem`. Which naming should the new methods follow? The `decimal_downscale` benchmarks in #11426 cover `make_downscaler`, and the 34-digit `Decimal128` and 72-digit `Decimal256` `div` cases in #11427 reach `scaled_div`. ### Describe alternatives you've considered The primitive integer types could keep the default implementations. Whether they gain from an override is open until a benchmark shows it. ### Additional context This depends on #11425. -- 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]
