tustvold commented on PR #7159: URL: https://github.com/apache/arrow-rs/pull/7159#issuecomment-2685883823
> @tustvold Sorry, what was the reason for not proceeding with this change? I think we can remove div_wrapping from this PR, but rem_wrapping is still a valid case? I am trying to be pragmatic and avoid adding codegen for what is, I hope you will concede, an exceptionally niche edge case. That being said unless I am mistaken the only distinction between wrapping_rem and checked_rem is that for signed integers, dividing the smallest representable quantity by -1 results in "overflow", this is really an implementation detail of rem and is because the quotient overflows, but Rust doesn't distinguish this case. Given this what do you think of changing the behaviour of `Operator::Rem` to use the WrappingRem logic added in this PR, AFAICT the only difference is it now returns an arguably more correct result for this single case. I think this could reasonably be filed as a bug fix -- 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]
