david-mollitor-db commented on PR #58906: URL: https://github.com/apache/spark/pull/58906#issuecomment-5766108443
One more note for the record, in the interest of transparency: My initial implementation dropped the second `% n` unconditionally, and the entire existing unit-test suite still passed — none of the pre-existing cases exercised the negative-dividend / negative-divisor path (`r < 0`, `n < 0`), which is exactly where that shortcut is wrong (`pmod(-3, -5)` would return `-8` instead of the released `-3`). I've corrected the implementation to keep the modulo when the divisor is negative, and added regression cases in `ArithmeticExpressionSuite` for that path. These negative-divisor inputs are unlikely in practice — the primary internal caller, `HashPartitioning`, always passes a positive divisor (`numPartitions`) — but they're reachable through the `pmod` SQL function, so I added the coverage to lock in the existing behavior regardless. -- 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]
