This is an automated email from the ASF dual-hosted git repository.
LuciferYang pushed a commit to branch branch-4.0
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-4.0 by this push:
new c354d5bcea9a [SPARK-57506][SQL] Fix RTRIM-collation trimRight dropping
trailing spaces for strings with supplementary characters
c354d5bcea9a is described below
commit c354d5bcea9a94982badcf70ba6da87e7776c986
Author: YangJie <[email protected]>
AuthorDate: Fri Jun 19 21:39:07 2026 +0800
[SPARK-57506][SQL] Fix RTRIM-collation trimRight dropping trailing spaces
for strings with supplementary characters
### What changes were proposed in this pull request?
`CollationAwareUTF8String.trimRight` β the ICU path used by RTRIM-modifier
collations β compared a Java-String (UTF-16) index against a Unicode code-point
count: `lastNonSpacePosition == srcString.numChars()`. `lastNonSpacePosition`
is a UTF-16 index into `src = srcString.toValidString()` (it is initialized to
`src.length()` and decremented via `src.charAt`), so the sentinel must be
compared against `src.length()`, matching the `charIndex == src.length()` check
immediately above it. T [...]
### Why are the changes needed?
For RTRIM-style collations, trailing spaces are ignored while matching the
trim characters but must be re-appended to the result. With the code-point
count, the "no trailing spaces were skipped" sentinel fired spuriously whenever
the number of supplementary characters equaled the number of trailing spaces,
so the preserved trailing spaces were dropped. For example, under
`UNICODE_RTRIM`, right-trimming a supplementary character (such as U+1D538)
followed by a single space returned an [...]
### Does this PR introduce _any_ user-facing change?
Yes, it fixes incorrect results. Under RTRIM-modifier collations,
right-trim on a string containing supplementary characters followed by trailing
spaces (when the count of supplementary characters equals the count of trailing
spaces) no longer drops the preserved trailing spaces. Only
previously-incorrect results change.
### How was this patch tested?
Added cases to `CollationSupportSuite#testStringTrimRight` covering
RTRIM-modifier collations across all three trim paths β `UTF8_BINARY_RTRIM`,
`UTF8_LCASE_RTRIM`, and `UNICODE_RTRIM`. For each input: supplementary
characters with trailing spaces (including the two coincidence cases that
previously returned the wrong value on the ICU path), a BMP control, a
non-coincidence control, and the all-spaces early-return path. A separate
case-folding case (`xB`/`b`) confirms only `UTF8_LCASE [...]
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 4.8)
Closes #56567 from LuciferYang/collation-trimright-rtrim-fix.
Authored-by: YangJie <[email protected]>
Signed-off-by: yangjie01 <[email protected]>
(cherry picked from commit d5e87fdc6c18184e00e3b8492f94aeaffe552681)
Signed-off-by: yangjie01 <[email protected]>
---
.../catalyst/util/CollationAwareUTF8String.java | 2 +-
.../spark/unsafe/types/CollationSupportSuite.java | 26 ++++++++++++++++++++++
2 files changed, 27 insertions(+), 1 deletion(-)
diff --git
a/common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationAwareUTF8String.java
b/common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationAwareUTF8String.java
index e455e531de0d..523606cb7837 100644
---
a/common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationAwareUTF8String.java
+++
b/common/unsafe/src/main/java/org/apache/spark/sql/catalyst/util/CollationAwareUTF8String.java
@@ -1519,7 +1519,7 @@ public class CollationAwareUTF8String {
if (charIndex == src.length()) {
return srcString;
}
- if (lastNonSpacePosition == srcString.numChars()) {
+ if (lastNonSpacePosition == src.length()) {
return UTF8String.fromString(src.substring(0, charIndex));
}
return UTF8String.fromString(
diff --git
a/common/unsafe/src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java
b/common/unsafe/src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java
index 1db163c1c822..6372d7e4663c 100644
---
a/common/unsafe/src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java
+++
b/common/unsafe/src/test/java/org/apache/spark/unsafe/types/CollationSupportSuite.java
@@ -3647,6 +3647,32 @@ public class CollationSupportSuite {
assertStringTrimRight(UTF8_LCASE, "πΈ", "a", "πΈ");
assertStringTrimRight(UNICODE, "πΈ", "a", "πΈ");
assertStringTrimRight(UNICODE_CI, "πΈ", "a", "");
+ // RTRIM-modifier collations ignore trailing spaces while matching the
trim characters, then
+ // re-append them. The behaviour must agree across the UTF8_BINARY
(binaryTrimRight),
+ // UTF8_LCASE (lowercaseTrimRight), and ICU (trimRight) paths. The
supplementary-character
+ // cases below (trailing-space count == supplementary code-point count)
regressed on the ICU
+ // path before SPARK-57506, which compared a Java-char index against a
code-point count.
+ assertStringTrimRight("UTF8_BINARY_RTRIM", "x ", "x", " ");
+ assertStringTrimRight("UTF8_LCASE_RTRIM", "x ", "x", " ");
+ assertStringTrimRight("UNICODE_RTRIM", "x ", "x", " ");
+ assertStringTrimRight("UTF8_BINARY_RTRIM", " ", "x", " ");
+ assertStringTrimRight("UTF8_LCASE_RTRIM", " ", "x", " ");
+ assertStringTrimRight("UNICODE_RTRIM", " ", "x", " ");
+ assertStringTrimRight("UTF8_BINARY_RTRIM", "πΈ ", "πΈ", " ");
+ assertStringTrimRight("UTF8_LCASE_RTRIM", "πΈ ", "πΈ", " ");
+ assertStringTrimRight("UNICODE_RTRIM", "πΈ ", "πΈ", " ");
+ assertStringTrimRight("UTF8_BINARY_RTRIM", "πΈ ", "πΈ", " ");
+ assertStringTrimRight("UTF8_LCASE_RTRIM", "πΈ ", "πΈ", " ");
+ assertStringTrimRight("UNICODE_RTRIM", "πΈ ", "πΈ", " ");
+ assertStringTrimRight("UTF8_BINARY_RTRIM", "πΈπΈ ", "πΈ", " ");
+ assertStringTrimRight("UTF8_LCASE_RTRIM", "πΈπΈ ", "πΈ", " ");
+ assertStringTrimRight("UNICODE_RTRIM", "πΈπΈ ", "πΈ", " ");
+ // Case-folding interacts with space preservation per path: only
UTF8_LCASE folds B to b, so
+ // only it trims the trailing 'B' and re-appends the space; binary and
(case-sensitive) ICU
+ // leave the input unchanged. This exercises the lcase space-preservation
branch on its own.
+ assertStringTrimRight("UTF8_BINARY_RTRIM", "xB ", "b", "xB ");
+ assertStringTrimRight("UTF8_LCASE_RTRIM", "xB ", "b", "x ");
+ assertStringTrimRight("UNICODE_RTRIM", "xB ", "b", "xB ");
}
/**
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
