sepuri sai krishna created SPARK-58713:
------------------------------------------
Summary: overlay() returns wrong results for out-of-range position
arguments
Key: SPARK-58713
URL: https://issues.apache.org/jira/browse/SPARK-58713
Project: Spark
Issue Type: Bug
Components: SQL
Affects Versions: 4.2.0
Reporter: sepuri sai krishna
{{Overlay.calculate}} computes its two slice boundaries in {{int}} arithmetic,
so a position argument near the ends of the {{int}} range wraps and the
function silently returns a wrong result. Both the STRING and the BINARY
overload are affected, on the interpreted and the codegen path, with default
configuration.
{code:scala}
def calculate(input: UTF8String, replace: UTF8String, pos: Int, len: Int):
UTF8String = {
val builder = new UTF8StringBuilder
builder.append(input.substringSQL(1, pos - 1)) // (1) `pos - 1`
overflows
builder.append(replace)
val length = if (len >= 0) len else replace.numChars
builder.append(input.substringSQL(pos + length, Int.MaxValue)) // (2) `pos +
length` overflows
builder.build()
}
{code}
The BINARY overload has the same two expressions, via
{{ByteArray.subStringSQL}}.
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. In both
cases the input is duplicated around the replacement.
h3. Reproduction
{code:sql}
SELECT overlay('Spark SQL' PLACING '_' FROM 2147483647 FOR 5);
-- actual: 'Spark SQL_Spark SQL'
-- expected: 'Spark SQL_'
SELECT overlay('Spark SQL' PLACING '_' FROM -2147483648 FOR 1);
-- actual: 'Spark SQL_Spark SQL'
-- expected: '_Spark SQL'
{code}
Measured on current master, input {{'Spark SQL'}} and replacement {{'_'}}:
|| pos || len || actual (STRING) || actual (BINARY) || expected ||
| {{Int.MaxValue}} | 5 | {{'Spark SQL_Spark SQL'}} | {{'Spark SQL_Spark SQL'}}
| {{'Spark SQL_'}} |
| {{Int.MaxValue}} | -1 | {{'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}} | -1 | {{'Spark SQL_Spark SQL'}} | {{'Spark SQL_Spark SQL'}}
| {{'_Spark SQL'}} |
All in-range positions are unaffected; {{pos}} values such as 2, 6, 7, 100 and
-1 produce identical results before and after the fix.
Note the STRING and BINARY columns already disagree for {{pos = Int.MaxValue,
len = -1}}, which is a symptom of the separate clamping problem fixed under
SPARK-58708. That fix brings BINARY into line with STRING for that row, but
both remain wrong until the arithmetic here is corrected -- the overflow
happens in {{Overlay.calculate}} before {{substringSQL}} is ever reached, so
SPARK-58708 does not and cannot fix it.
h3. Proposed fix
Compute both boundaries in {{long}} and clamp into the {{int}} range before
calling {{substringSQL}}:
{code:scala}
private def clamp(v: Long): Int =
math.max(Int.MinValue, math.min(Int.MaxValue, v)).toInt
{code}
applied to {{pos - 1}} and {{pos + length}} in both overloads. Clamping rather
than throwing keeps the behaviour consistent with {{substring()}}, which
saturates rather than erroring for out-of-range positions. With this change all
five rows above produce the expected column, and the in-range cases are
unchanged.
h3. Impact
Wrong results, silently. No error is raised and no configuration is involved.
The affected argument range is narrow, so the practical exposure is low, but
the failure mode is a duplicated input rather than an obviously bogus value,
which makes it hard to notice.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
