Re: [PR] [SPARK-57506][SQL] Fix RTRIM-collation trimRight dropping trailing spaces for strings with supplementary characters [spark]

Wed, 17 Jun 2026 22:58:45 -0700 


LuciferYang commented on code in PR #56567:
URL: https://github.com/apache/spark/pull/56567#discussion_r3433432137


##########
common/unsafe/src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java:
##########
@@ -3647,6 +3647,15 @@ public void testStringTrimRight() throws SparkException {
     assertStringTrimRight(UTF8_LCASE, "𝔸", "a", "𝔸");
     assertStringTrimRight(UNICODE, "𝔸", "a", "𝔸");
     assertStringTrimRight(UNICODE_CI, "𝔸", "a", "");
+    // RTRIM-modifier collations (ICU path): trailing spaces are ignored while 
matching but must
+    // be re-appended afterwards. When the number of trailing spaces equals 
the number of
+    // supplementary code points, a Java-char-index vs code-point-count 
comparison previously
+    // dropped the preserved spaces.
+    assertStringTrimRight("UNICODE_RTRIM", "x ", "x", " ");
+    assertStringTrimRight("UNICODE_RTRIM", "   ", "x", "   ");
+    assertStringTrimRight("UNICODE_RTRIM", "𝔸 ", "𝔸", " ");
+    assertStringTrimRight("UNICODE_RTRIM", "𝔸  ", "𝔸", "  ");
+    assertStringTrimRight("UNICODE_RTRIM", "𝔸𝔸  ", "𝔸", "  ");
   }

Review Comment:
   Done β€” added `UTF8_BINARY_RTRIM` and `UTF8_LCASE_RTRIM` next to 
`UNICODE_RTRIM` for each input, grouped per input so the cross-collation 
agreement is visible. These two dispatch to `binaryTrimRight` / 
`lowercaseTrimRight` (not the ICU `trimRight` this PR fixes); both index by 
code point throughout, so they were already correct and the new cases pass. 
Also added a case-folding case (`xB` / `b`) where `UTF8_LCASE` diverges from 
binary/ICU, so the lcase path can't silently regress to the binary one.



-- 
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]

Reply via email to