uros-b commented on PR #57890:
URL: https://github.com/apache/spark/pull/57890#issuecomment-5238283347
Good catch, thanks. You are right that the description called this "the most
error-prone arithmetic in the file" and then left its sharpest edge
undocumented.
Added a test in a second commit. I confirmed the behaviour rather than
assuming which exception fires: `rightPad("a", 5, "")` throws
`ArithmeticException: / by zero`, from the unguarded `(width - str.length) /
s.length` divisor.
The test documents the crash rather than guarding it, and also pins the
three neighbouring cases where the divisor is never reached, since that is what
makes the failure conditional rather than universal:
```scala
test("rightPad with an empty pad string fails when padding is required") {
intercept[ArithmeticException] {
SparkStringUtils.rightPad("a", 5, "")
}
assert(SparkStringUtils.rightPad("hello", 5, "") === "hello")
assert(SparkStringUtils.rightPad("hello", 3, "") === "hello")
assert(SparkStringUtils.rightPad(null, 5, "") === null)
}
```
I deliberately did not add a guard. Changing the method to tolerate an empty
pad string would be a behaviour change to a `private[spark]` helper in a
test-only PR, and it is not obvious what the right result would be (returning
`str` unpadded, or throwing `IllegalArgumentException` instead). Happy to
follow up separately if you would prefer the method rejected an empty pad
string explicitly.
Suite now runs 7 tests, all passing.
--
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]
