HyukjinKwon commented on code in PR #59346:
URL: https://github.com/apache/spark/pull/59346#discussion_r4256294219
##########
common/unsafe/src/test/java/org/apache/spark/unsafe/types/UTF8StringSuite.java:
##########
@@ -388,6 +388,10 @@ public void indexOf() {
assertEquals(0, fromString("hello").indexOf(EMPTY_UTF8, 1, 1));
assertEquals(0, fromString("hello").indexOf(EMPTY_UTF8, 5, 1));
assertEquals(0, fromString("hello").indexOf(EMPTY_UTF8, -1, 1));
+ assertEquals(-1, fromString("hello").indexOf(EMPTY_UTF8, 0, 1));
Review Comment:
**Nit (P3):** The comment above this block still says `// Empty substring
(behavior depends on indexOfEmpty, currently returns 0)`, but these four new
assertions expect `-1` and never reach `indexOfEmpty`, because the zero-start
check now runs first. Could you give them their own one-line comment, for
example `// Zero start returns -1 regardless of substring`? That also keeps
them from being changed by mistake when SPARK-48284 later updates the
`indexOfEmpty` cases above.
**Verification:**
- **Inspection:** Each test section comment in the empty-substring and
zero-start block of the UTF8String indexOf tests matches the expected values of
the assertions grouped under it.
--
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]
