From af3091baffc97109baefcdb1d0b7cc13e776c8a3 Mon Sep 17 00:00:00 2001 From: Eric Yang Date: Thu, 30 Jul 2026 00:02:52 -0700 Subject: [PATCH 1/2] [SPARK-58443][SQL] Fix instr returning a wrong position for Int.MinValue start due to negation overflow in UTF8String.indexOf --- .../src/main/java/org/apache/spark/unsafe/types/UTF8String.java | 2 ++ .../java/org/apache/spark/unsafe/types/UTF8StringSuite.java | 2 ++ 2 files changed, 4 insertions(+) diff --git a/common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java b/common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java index 03c42785edb28..fdc46cff712a4 100644 --- a/common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java +++ b/common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java @@ -1300,6 +1300,8 @@ public int indexOf(UTF8String pattern, int start, int occurrence) { } else { // For negative start, skip |start| characters from the end to position // byteIdx at the starting byte of the first character to compare. + // -start would overflow for Int.MinValue; such a start is always out of range. + if (start == Integer.MIN_VALUE) return -1; charsToSkip = -start; byteIdx = numBytes; while (byteIdx > 0 && charCount < charsToSkip) { diff --git a/common/unsafe/src/test/java/org/apache/spark/unsafe/types/UTF8StringSuite.java b/common/unsafe/src/test/java/org/apache/spark/unsafe/types/UTF8StringSuite.java index 420e49d0a26e3..e6e3346760cc1 100644 --- a/common/unsafe/src/test/java/org/apache/spark/unsafe/types/UTF8StringSuite.java +++ b/common/unsafe/src/test/java/org/apache/spark/unsafe/types/UTF8StringSuite.java @@ -332,6 +332,8 @@ public void indexOf() { assertEquals(-1, fromString("hello").indexOf(fromString("o"), 6, 1)); assertEquals(0, fromString("hello").indexOf(fromString("h"), -5, 1)); assertEquals(-1, fromString("hello").indexOf(fromString("h"), -6, 1)); + // SPARK-58443: Int.MinValue start must not overflow into a backward search from the end. + assertEquals(-1, fromString("abcabc").indexOf(fromString("abc"), Integer.MIN_VALUE, 1)); // Target larger than string assertEquals(-1, fromString("ab").indexOf(fromString("abc"), 1, 1)); From 6ecd81c4da3ed3a7c03fcfb8824afb8d27202665 Mon Sep 17 00:00:00 2001 From: Eric Yang Date: Fri, 31 Jul 2026 15:12:50 -0700 Subject: [PATCH 2/2] [SPARK-58443][SQL] Extend the Int.MinValue overflow fix to substring_index and all collations --- .../catalyst/util/CollationAwareUTF8String.java | 12 +++++++----- .../org/apache/spark/unsafe/types/UTF8String.java | 15 ++++++++------- .../spark/unsafe/types/CollationSupportSuite.java | 12 ++++++++++++ .../spark/unsafe/types/UTF8StringSuite.java | 6 +++++- .../expressions/StringExpressionsSuite.scala | 14 ++++++++++++++ 5 files changed, 46 insertions(+), 13 deletions(-) diff --git a/common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationAwareUTF8String.java b/common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationAwareUTF8String.java index 0da7af3ca6407..5e239212dfc66 100644 --- a/common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationAwareUTF8String.java +++ b/common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationAwareUTF8String.java @@ -979,7 +979,7 @@ private static int findIndex(final StringSearch stringSearch, int count) { return index; } - private static int findIndexReverse(final StringSearch stringSearch, int count) { + private static int findIndexReverse(final StringSearch stringSearch, long count) { assert(count >= 0); int index = 0; while (count > 0) { @@ -1026,7 +1026,8 @@ public static UTF8String subStringIndex(final UTF8String string, final UTF8Strin } } else { // If the count is negative, we search for the count-th delimiter from the right. - int searchIndex = findIndexReverse(stringSearch, -count); + // The count is widened to a long because negating it overflows for Integer.MIN_VALUE. + int searchIndex = findIndexReverse(stringSearch, -(long) count); if (searchIndex == MATCH_NOT_FOUND) { return string; } else if (searchIndex == str.length()) { @@ -1057,10 +1058,11 @@ public static UTF8String lowercaseSubStringIndex(final UTF8String string, } else { // Search right to left (note: the end code point is exclusive). int matchLength = string.numChars() + 1; - count = -count; - while (count > 0) { + // `remaining` is a long because negating `count` overflows for Integer.MIN_VALUE. + long remaining = -(long) count; + while (remaining > 0) { matchLength = lowercaseRFind(string, lowercaseDelimiter, matchLength - 1); - if (matchLength > MATCH_NOT_FOUND) --count; // Found a delimiter. + if (matchLength > MATCH_NOT_FOUND) --remaining; // Found a delimiter. else return string; // Cannot find enough delimiters in the string. } return string.substring(matchLength, string.numChars()); diff --git a/common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java b/common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java index fdc46cff712a4..a9d3933816979 100644 --- a/common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java +++ b/common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java @@ -1289,7 +1289,9 @@ public int indexOf(UTF8String pattern, int start, int occurrence) { } int charCount = 0; - int charsToSkip, byteIdx; + int byteIdx; + // `charsToSkip` is a long because negating `start` overflows for Integer.MIN_VALUE. + long charsToSkip; if (start > 0) { byteIdx = 0; // position in byte charsToSkip = start - 1; // skip character count @@ -1300,9 +1302,7 @@ public int indexOf(UTF8String pattern, int start, int occurrence) { } else { // For negative start, skip |start| characters from the end to position // byteIdx at the starting byte of the first character to compare. - // -start would overflow for Int.MinValue; such a start is always out of range. - if (start == Integer.MIN_VALUE) return -1; - charsToSkip = -start; + charsToSkip = -(long) start; byteIdx = numBytes; while (byteIdx > 0 && charCount < charsToSkip) { byteIdx = prevCharStart(byteIdx); @@ -1454,11 +1454,12 @@ public UTF8String subStringIndex(UTF8String delim, int count) { } else { int idx = numBytes - delim.numBytes + 1; - count = -count; - while (count > 0) { + // `remaining` is a long because negating `count` overflows for Integer.MIN_VALUE. + long remaining = -(long) count; + while (remaining > 0) { idx = rfind(delim, idx - 1); if (idx >= 0) { - count --; + remaining --; } else { // can not find enough delim return this; diff --git a/common/unsafe/src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java b/common/unsafe/src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java index 9e7cf3893185e..7f3c1a925e026 100644 --- a/common/unsafe/src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java +++ b/common/unsafe/src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java @@ -2737,6 +2737,10 @@ public void testSubstringIndex() throws SparkException { assertSubstringIndex("a🙃b🙃c", "d", -1, UTF8_LCASE, "a🙃b🙃c"); assertSubstringIndex("a🙃b🙃c", "d", -1, UNICODE, "a🙃b🙃c"); assertSubstringIndex("a🙃b🙃c", "d", -1, UNICODE_CI, "a🙃b🙃c"); + assertSubstringIndex("a.b.c", ".", Integer.MIN_VALUE, UTF8_BINARY, "a.b.c"); + assertSubstringIndex("a.b.c", ".", Integer.MIN_VALUE, UTF8_LCASE, "a.b.c"); + assertSubstringIndex("a.b.c", ".", Integer.MIN_VALUE, UNICODE, "a.b.c"); + assertSubstringIndex("a.b.c", ".", Integer.MIN_VALUE, UNICODE_CI, "a.b.c"); } /** @@ -4192,6 +4196,14 @@ public void testStringInstrWithOccurrence() throws SparkException { // Boundary: start out of range (forward/backward) assertStringInstrWithOccurrence("İoi\u0307oİo", "İo", 10, 1, UTF8_LCASE, 0); assertStringInstrWithOccurrence("İoi\u0307oİo", "İo", -10, 1, UNICODE_CI, 0); + assertStringInstrWithOccurrence("abcabc", "abc", Integer.MIN_VALUE, 1, UTF8_BINARY, 0); + assertStringInstrWithOccurrence("abcabc", "abc", Integer.MIN_VALUE, 1, UTF8_LCASE, 0); + assertStringInstrWithOccurrence("abcabc", "abc", Integer.MIN_VALUE, 1, UNICODE, 0); + assertStringInstrWithOccurrence("abcabc", "abc", Integer.MIN_VALUE, 1, UNICODE_CI, 0); + assertStringInstrWithOccurrence("İoi\u0307oİo", "İo", Integer.MIN_VALUE, 1, UTF8_BINARY, 0); + assertStringInstrWithOccurrence("İoi\u0307oİo", "İo", Integer.MIN_VALUE, 1, UTF8_LCASE, 0); + assertStringInstrWithOccurrence("İoi\u0307oİo", "İo", Integer.MIN_VALUE, 1, UNICODE, 0); + assertStringInstrWithOccurrence("İoi\u0307oİo", "İo", Integer.MIN_VALUE, 1, UNICODE_CI, 0); String sigmaStr = "σΣςσΣς"; // 1:σ, 2:Σ, 3:ς, 4:σ, 5:Σ, 6:ς // UTF8_BINARY: all sigma forms are distinct, only exact byte matches succeed assertStringInstrWithOccurrence("σΣςσΣς", "Σ", 1, 2, UTF8_BINARY, 5); diff --git a/common/unsafe/src/test/java/org/apache/spark/unsafe/types/UTF8StringSuite.java b/common/unsafe/src/test/java/org/apache/spark/unsafe/types/UTF8StringSuite.java index e6e3346760cc1..9d67b7b611b19 100644 --- a/common/unsafe/src/test/java/org/apache/spark/unsafe/types/UTF8StringSuite.java +++ b/common/unsafe/src/test/java/org/apache/spark/unsafe/types/UTF8StringSuite.java @@ -332,8 +332,8 @@ public void indexOf() { assertEquals(-1, fromString("hello").indexOf(fromString("o"), 6, 1)); assertEquals(0, fromString("hello").indexOf(fromString("h"), -5, 1)); assertEquals(-1, fromString("hello").indexOf(fromString("h"), -6, 1)); - // SPARK-58443: Int.MinValue start must not overflow into a backward search from the end. assertEquals(-1, fromString("abcabc").indexOf(fromString("abc"), Integer.MIN_VALUE, 1)); + assertEquals(-1, fromString("abcabc").indexOf(fromString("abc"), Integer.MIN_VALUE + 1, 1)); // Target larger than string assertEquals(-1, fromString("ab").indexOf(fromString("abc"), 1, 1)); @@ -383,6 +383,10 @@ public void substring_index() { // overlapped delim assertEquals(fromString("||"), fromString("||||||").subStringIndex(fromString("|||"), 3)); assertEquals(fromString("|||"), fromString("||||||").subStringIndex(fromString("|||"), -4)); + assertEquals(fromString("www.apache.org"), + fromString("www.apache.org").subStringIndex(fromString("."), Integer.MIN_VALUE)); + assertEquals(fromString("www.apache.org"), + fromString("www.apache.org").subStringIndex(fromString("."), Integer.MIN_VALUE + 1)); } @Test diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/StringExpressionsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/StringExpressionsSuite.scala index 711b5edd72ad1..adcfb2a23996d 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/StringExpressionsSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/StringExpressionsSuite.scala @@ -346,6 +346,14 @@ class StringExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper { SubstringIndex(Literal("www.apache.org"), Literal("."), Literal(-2)), "apache.org") checkEvaluation( SubstringIndex(Literal("www.apache.org"), Literal("."), Literal(-1)), "org") + // SPARK-58443: negating a count of Integer.MIN_VALUE overflows back to Integer.MIN_VALUE; + // such a count is always out of range, so the whole string is returned. + checkEvaluation( + SubstringIndex(Literal("www.apache.org"), Literal("."), Literal(Int.MinValue)), + "www.apache.org") + checkEvaluation( + SubstringIndex(Literal("www.apache.org"), Literal("."), Literal(Int.MinValue + 1)), + "www.apache.org") checkEvaluation( SubstringIndex(Literal(""), Literal("."), Literal(-2)), "") checkEvaluation( @@ -1064,6 +1072,12 @@ class StringExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper { Literal("abcabc"), Literal("ab"), Literal(-2), Literal(3)), 0) checkEvaluation(StringInstrWithOccurrence( Literal("abcabc"), Literal("ab"), Literal(-3), Literal(3)), 0) + // SPARK-58443: negating a start of Integer.MIN_VALUE overflows back to Integer.MIN_VALUE; + // such a start is always out of range, so no match is reported. + checkEvaluation(StringInstrWithOccurrence( + Literal("abcabc"), Literal("abc"), Literal(Int.MinValue), Literal(1)), 0) + checkEvaluation(StringInstrWithOccurrence( + Literal("abcabc"), Literal("abc"), Literal(Int.MinValue + 1), Literal(1)), 0) checkEvaluation(StringInstrWithOccurrence( Literal("abc"), Literal("b"), Literal(0), Literal(1)), 0) checkEvaluation(StringInstrWithOccurrence(