SEPURI-SAI-KRISHNA opened a new pull request, #57931:
URL: https://github.com/apache/spark/pull/57931
### What changes were proposed in this pull request?
`Overlay.calculate` computes its two slice boundaries in `int` arithmetic:
```scala
builder.append(input.substringSQL(1, pos - 1)) // (1) `pos
- 1` overflows
...
builder.append(input.substringSQL(pos + length, Int.MaxValue)) // (2) `pos
+ length` overflows
```
The BINARY overload has the same two expressions, via
`ByteArray.subStringSQL`.
This PR computes both boundaries as `long` and saturates them into the `int`
range before
calling `substringSQL`, through a small private helper:
```scala
private def clamp(value: Long): Int =
math.max(Int.MinValue.toLong, math.min(Int.MaxValue.toLong, value)).toInt
```
Saturating rather than throwing keeps `overlay()` consistent with
`substring()`, which already
treats out-of-range positions by clamping. Only the four boundary
expressions change; the rest of
both overloads is untouched.
### Why are the changes needed?
For `pos == Int.MinValue`, `pos - 1` wraps to `Int.MaxValue`, so the leading
slice becomes the
whole input instead of the empty string. For `pos` near `Int.MaxValue`, `pos
+ length` wraps to a
large negative value, which `substringSQL` interprets as an offset from the
end of the input, so
the trailing slice becomes the whole input instead of the empty string.
Either way the input is
duplicated around the replacement, silently and with no error raised.
```sql
SELECT overlay('Spark SQL' PLACING '_' FROM 2147483647 FOR 5);
-- before: 'Spark SQL_Spark SQL'
-- after: 'Spark SQL_'
SELECT overlay('Spark SQL' PLACING '_' FROM -2147483648 FOR 1);
-- before: 'Spark SQL_Spark SQL'
-- after: '_Spark SQL'
```
Both the STRING and the BINARY overload are affected, on the interpreted and
the codegen path,
with default configuration.
### Does this PR introduce _any_ user-facing change?
Yes, for positions near the ends of the `int` range, which are currently
wrong. Measured with
input `'Spark SQL'` and replacement `'_'`:
| pos | len | before (STRING) | before (BINARY) | after (both) |
|---|---|---|---|---|
| `Int.MaxValue` | 5 | `Spark SQL_Spark SQL` | `Spark SQL_Spark SQL` |
`Spark SQL_` |
| `Int.MaxValue` | default | `Spark SQL_Spark SQ` | `Spark SQL_Spark SQL` |
`Spark SQL_` |
| `Int.MaxValue - 2` | 10 | `Spark SQL_Spark SQL` | `Spark SQL_Spark SQL` |
`Spark SQL_` |
| `Int.MinValue` | 1 | `Spark SQL_Spark SQL` | `Spark SQL_Spark SQL` |
`_Spark SQL` |
| `Int.MinValue` | default | `Spark SQL_Spark SQL` | `Spark SQL_Spark SQL` |
`_Spark SQL` |
No change for any in-range position. The existing test cases covering
positions 0, 2, 6, 7, 10 and
-10 pass unmodified.
Note the STRING and BINARY columns already disagree on the second row. That
is a symptom of a
separate clamping problem in `ByteArray.subStringSQL`, fixed under
SPARK-58708 (#57922). The two
are independent: the overflow addressed here happens in `Overlay.calculate`
before `substringSQL`
is ever reached, so neither PR fixes the other's bug, and they can be merged
in either order.
### How was this patch tested?
Five new cases added to each of the existing `overlay for string` and
`overlay for byte array`
tests in `StringExpressionsSuite`, covering both overflow directions and
both the two-argument
(default length) and three-argument forms. These go through
`checkEvaluation`, so the interpreted
and codegen paths are both exercised.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5)
--
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]
