HyukjinKwon commented on code in PR #59198:
URL: https://github.com/apache/spark/pull/59198#discussion_r4166090199
##########
common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java:
##########
@@ -821,8 +808,36 @@ public UTF8String toUpperCase() {
return isFullAscii() ? toUpperCaseAscii() : toUpperCaseSlow();
}
+ /**
+ * Returns the upper case of this ASCII string. The caller must ensure the
string is full
+ * ASCII; the normal entry points reach this only after {@link
#isFullAscii()} returns true.
+ *
+ * The result array is allocated lazily, only once a byte actually changes
case: a string
+ * that is already upper case returns {@code this} with no allocation.
Because upper-casing
+ * ASCII stays ASCII, the result is tagged full ASCII so later operations
can skip re-scanning.
+ */
public UTF8String toUpperCaseAscii() {
- return convertAscii(Character::toUpperCase);
+ assert isFullAscii();
+ byte[] bytes = null;
+ for (int i = 0; i < numBytes; i++) {
+ byte b = getByte(i);
+ byte upper = (b >= 'a' && b <= 'z') ? (byte) (b - 32) : b;
+ if (upper != b && bytes == null) {
+ // First byte to change case: allocate the result and copy the clean
prefix [0, i).
+ bytes = new byte[numBytes];
+ copyMemory(base, offset, bytes, BYTE_ARRAY_OFFSET, i);
Review Comment:
**Nit (P3):** This new lazy-allocation branch bulk-copies the
already-correct prefix `[0, i)` with `copyMemory` before the first
case-changing byte, but no deterministic test exercises it with a non-empty
prefix. `UTF8StringSuite.upperAndLower` only uses full-flip ASCII inputs
(`ABCXYZ`/`abcxyz`, first change at index 0, so the prefix length is always 0)
and non-ASCII inputs that take the slow path, and
`UTF8StringPropertyCheckSuite` generates full-Unicode strings that essentially
never land on the ASCII fast path. A future off-by-one or wrong-offset
regression in this prefix copy would silently corrupt `lower()`/`upper()`
output for the common mixed-case input (e.g. `toUpperCase("ABCdef")`) with no
failing assertion. Consider adding a one-line assertion for a full-ASCII string
whose first case-changing byte is past index 0, for both methods -- e.g.
`assertEquals(fromString("ABCDEF"), fromString("ABCdef").toUpperCase())` and
its lowercase mirror.
**Verification:**
- **Regression:** A unit assertion that corrupting the preserved clean
prefix (wrong copy length/offset) would fail: for an ASCII input such as an
already-uppercase prefix + lowercase suffix, toUpperCase returns the full
uppercase string, and the lowercase mirror holds.
--
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]
