SEPURI-SAI-KRISHNA commented on PR #57922:
URL: https://github.com/apache/spark/pull/57922#issuecomment-5250433717
Pushed a small follow-up commit.
**Removed the `pos > bytes.length` early return in `subStringSQL`.** It was
load-bearing before this change, but is not any more: if `pos > bytes.length`
then `start = pos - 1 >= bytes.length`, so the clamped `end = min(start + len,
bytes.length)` is always `<= start` and the `start >= end` check below already
returns `EMPTY_BYTE`. I checked that rather than
just reasoning about it -- comparing the implementation with and without the
guard over the 25x25 boundary grid for input lengths 0..12, plus 3M random
cases with `pos` biased to `n`, `n+1` and `n+4`, gives zero differences.
One thing worth flagging for reviewers looking at the call sites:
**`Overlay` has the same class of overflow and is not fixed here.**
`Overlay.calculate` computes `pos - 1` and `pos + length` in `int`, so:
```sql
SELECT overlay('Spark SQL' PLACING '_' FROM 2147483647 FOR 5);
-- returns 'Spark SQL_Spark SQL', expected 'Spark SQL_'
```
This affects the STRING and BINARY overloads equally, and is independent of
the clamping fixed here -- the overflow happens in `Overlay` before
`subStringSQL` is ever called. I kept it out of this PR to keep the change
focused, and will file it separately.
Note that because the BINARY overload of `Overlay` routes through
`ByteArray.subStringSQL`, this PR does change its output for those
already-broken overflowing arguments -- from one wrong answer to a different
wrong answer. No existing test covers `Overlay` with positions near the `int`
boundaries, and `StringExpressionsSuite` (which holds all of them) passes.
--
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]
