Skip to content

[SPARK-58443][SQL] Fix wrong results/exception in instr and substring_index for an Integer.MIN_VALUE position or count - #57648

Open
jiwen624 wants to merge 2 commits into
apache:masterfrom
jiwen624:SPARK-58443
Open

[SPARK-58443][SQL] Fix wrong results/exception in instr and substring_index for an Integer.MIN_VALUE position or count#57648
jiwen624 wants to merge 2 commits into
apache:masterfrom
jiwen624:SPARK-58443

Conversation

@jiwen624

@jiwen624 jiwen624 commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Four backward-search paths negate a user-supplied int and use the result as a number of characters or delimiters to skip. -Integer.MIN_VALUE wraps back to Integer.MIN_VALUE, so the value stays negative: the search loop never runs and the out-of-range check that should have fired is bypassed.

Each site now performs the negation in long (-(long) start / -(long) count), so Integer.MIN_VALUE stays positive after negation and flows through the same range checks and search loops as every other negative value. No value is special-cased, and the loops still terminate as soon as the string runs out of characters or delimiters.

Site Function Collations Behavior before
UTF8String.indexOf(pattern, start, occurrence) instr UTF8_BINARY, UTF8_LCASE (all-ASCII fast path) behaves as if start = -1 instead of returning 0
UTF8String.subStringIndex substring_index UTF8_BINARY NegativeArraySizeException
CollationAwareUTF8String.lowercaseSubStringIndex substring_index UTF8_LCASE empty string
CollationAwareUTF8String.subStringIndex substring_index ICU AssertionError under -ea; correct otherwise

The last row is latent: assertions are enabled in Spark's test JVMs but not in deployments, so it is hardening rather than a user-visible defect.

Why are the changes needed?

SELECT instr('abcabc', 'abc', -2147483648, 1);      -- returns 4, expected 0
SELECT substring_index('a.b.c', '.', -2147483648);  -- NegativeArraySizeException, expected a.b.c
SELECT substring_index(collate('a.b.c', 'UTF8_LCASE'), '.', -2147483648);  -- returns '', expected a.b.c

Does this PR introduce any user-facing change?

Yes - bug fix

How was this patch tested?

New test cases added

Was this patch authored or co-authored using generative AI tooling?

Yes

…lue start due to negation overflow in UTF8String.indexOf
@jiwen624 jiwen624 changed the title [SPARK-58443][SQL] Fix instr returning a wrong position for Int.MinValue start due to negation overflow in UTF8String.indexOf [SPARK-58443][SQL] Fix wrong results in instr and substring_index for an Int.MinValue position or count Jul 31, 2026
@jiwen624
jiwen624 force-pushed the SPARK-58443 branch 3 times, most recently from 9b69733 to b4709df Compare August 1, 2026 04:06
@jiwen624 jiwen624 changed the title [SPARK-58443][SQL] Fix wrong results in instr and substring_index for an Int.MinValue position or count [SPARK-58443][SQL] Fix wrong results/exception in instr and substring_index for an Int.MinValue position or count Aug 1, 2026
@jiwen624 jiwen624 changed the title [SPARK-58443][SQL] Fix wrong results/exception in instr and substring_index for an Int.MinValue position or count [SPARK-58443][SQL] Fix wrong results/exception in instr and substring_index for an Integer.MIN_VALUE position or count Aug 1, 2026
@jiwen624
jiwen624 marked this pull request as ready for review August 1, 2026 06:01
@uros-b

uros-b commented Aug 1, 2026

Copy link
Copy Markdown
Member

Thank you @jiwen624!

@dongjoon-hyun dongjoon-hyun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM.

@HyukjinKwon HyukjinKwon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 blocking, 0 non-blocking, 0 nits.
Correct, minimal integer-overflow edge fix applied consistently across the binary and ICU-collation paths.

Verification

Confirmed the overflow root cause (int negation of MIN_VALUE) and that widening to long before negation removes it, making MIN_VALUE behave like any other large-magnitude negative per the function contract. Applied identically in both string implementations; tests cover MIN_VALUE across UTF8_BINARY/LCASE/UNICODE/UNICODE_CI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants