david-mollitor-db opened a new pull request, #58906:
URL: https://github.com/apache/spark/pull/58906

   ### What changes were proposed in this pull request?
   
   The integral `MathUtils.pmod` overloads (`Int`, `Long`, `Byte`, `Short`) 
compute the positive
   remainder as:
   
   ```scala
   val r = a % n
   if (r < 0) (r + n) % n else r
   ```
   
   For integer types the trailing `% n` is redundant. `a % n` yields a value 
whose magnitude is
   strictly less than `|n|`, so when `r < 0` it lies in `(-|n|, 0)` and `r + n` 
is already in
   `[0, n)`. This PR removes the second modulo, leaving:
   
   ```scala
   val r = a % n
   if (r < 0) r + n else r
   ```
   
   The `Float`/`Double` overloads are left unchanged on purpose: floating-point 
rounding can round
   `r + n` up to exactly `n`, and the extra `% n` folds that back to `0`, so 
there it is a real
   correctness guard rather than a no-op.
   
   ### Why are the changes needed?
   
   `MathUtils.pmod` backs the `pmod` SQL function and the per-row partition-id 
expression of
   `HashPartitioning` (`Pmod(Murmur3Hash(...), numPartitions)`), so it runs 
once per row on the map
   side of every hash-partitioned shuffle. The trailing `% n` is dead 
computation. Removing it is a
   small readability and efficiency cleanup: besides skipping an operation, it 
lets the JIT compile
   the function to branchless, single-division code (the second modulo 
otherwise sits in a branch
   that cannot be turned into a conditional move).
   
   ### Does this PR introduce _any_ user-facing change?
   
   No. The result is identical for all inputs; only redundant computation is 
removed.
   
   ### How was this patch tested?
   
   Existing `pmod` coverage (e.g. `ArithmeticExpressionSuite`) continues to 
apply. The old and new
   integral forms were additionally cross-checked for exact equality over 18M 
random inputs plus
   boundary values (including `Int.MinValue` / `Long.MinValue`, `0`, `+/-1`, 
`+/-n`) across
   power-of-two and non-power-of-two divisors, with zero mismatches.
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   Generated-by: Isaac
   
   This pull request and its description were written by Isaac.
   


-- 
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]

Reply via email to