You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "jiwen624 (via GitHub)" <gi...@apache.org> on 2026/07/30 07:00:03 UTC
[PR] [SPARK-58441][SQL] Fix wrong results in instr and substring_index for ICU collations caused by inconsistent StringSearch iteration [spark]
jiwen624 opened a new pull request, #57647:
URL: https://github.com/apache/spark/pull/57647
### What changes were proposed in this pull request?
Fix two defects in `CollationAwareUTF8String`'s use of ICU `StringSearch`:
1. The backward-search helpers (`findStartIndexReverse` used by 3/4-arg `instr`, `findIndexReverse` used by `substring_index` with a negative count) enumerated matches with `StringSearch.previous()`. `previous()` does not visit the same match set as `next()`: it skips or misaligns matches when a character maps to multiple collation elements (any accented letter) and skips overlapping matches. The code base already documents this ICU inconsistency in `collationTrimRight` and avoids `previous()` there. Both helpers now enumerate matches forward with `next()` and select the requested occurrence from the end via a ring buffer.
2. The forward helper `findIndex` (`substring_index` positive count, `instr` forward search) detected a stuck overlapping iterator with `nextIndex == index && index != 0`. Since the "no match seen yet" sentinel was 0, a repeated match at position 0 (pattern starting with a surrogate pair) was counted once per occurrence; the recovery advance also skipped legitimate overlapping matches. The sentinel is now -1, and a stuck iterator advances by one code point instead of the full match length.
### Why are the changes needed?
`instr` and `substring_index` silently return wrong results under ICU collations on accented or emoji text:
```sql
SELECT instr(collate('bbébébé', 'UNICODE'), 'bé', -3, 1); -- returns 6, expected 4
SELECT instr(collate('babaéa', 'UNICODE'), 'aé', -3, 1); -- returns 0 (not found), expected 4
SELECT substring_index(collate('ééa', 'UNICODE'), 'é', -2); -- returns 'ééa', expected 'éa'
SELECT substring_index(collate('😀a😀b', 'UNICODE'), '😀', 2); -- returns '', expected '😀a'
```
UTF8_BINARY and UTF8_LCASE are unaffected (verified with a differential probe of ~1.27M input combinations against a code-point reference; all mismatches were confined to the ICU paths above).
### Does this PR introduce _any_ user-facing change?
Yes. bug fix.
### How was this patch tested?
Test cases added
### Was this patch authored or co-authored using generative AI tooling?
Yes
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-58441][SQL] Fix wrong results in instr and substring_index for ICU collations caused by inconsistent StringSearch iteration [spark]
Posted by "jiwen624 (via GitHub)" <gi...@apache.org>.
jiwen624 commented on PR #57647:
URL: https://github.com/apache/spark/pull/57647#issuecomment-5435578335
Hi @HyukjinKwon since you've reviewed it - may I know if you see any other concern or you think it can be approved & merged? Thank you very much.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org