This is an automated email from the ASF dual-hosted git repository.
cloud-fan pushed a commit to branch branch-4.x
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-4.x by this push:
new f1170fa59700 [SPARK-57747][SQL] Fix translate() to distinguish
deletion from literal U+0000 replacement
f1170fa59700 is described below
commit f1170fa597007896bcc2efc7a462413cc7410526
Author: Spyros Pavlatos <[email protected]>
AuthorDate: Thu Jul 16 21:45:31 2026 +0800
[SPARK-57747][SQL] Fix translate() to distinguish deletion from literal
U+0000 replacement
### What changes were proposed in this pull request?
`translate(input, from, to)` replaces characters and, when `to` is shorter
than `from`, deletes the extra matched characters. The deletion case was
signalled in-band by mapping those characters to the NUL character (`U+0000`)
and checking `"\0".equals(...)` in the execution loop. That in-band marker
collided with a legitimate replacement value: translating a character *to* a
literal `U+0000` was interpreted as deletion instead of a one-character NUL
replacement.
This PR switches the in-band deletion marker to the empty string, which can
never be a valid one-character replacement, and interprets dictionary values
uniformly:
- `null` mapping -> no mapping, keep the original character;
- empty-string mapping -> delete the character;
- non-empty mapping -> replace (including a literal `U+0000`).
The change is applied consistently across all translate paths so the
collations stay in sync:
- `StringTranslate.buildDict` (`stringExpressions.scala`) now uses `""` as
the deletion marker (and `!dict.containsKey(...)` for the first-occurrence-wins
guard);
- `UTF8String.translate` (UTF8_BINARY path);
- `CollationAwareUTF8String.lowercaseTranslate` (UTF8_LCASE path) and
`CollationAwareUTF8String.translate` (ICU path).
### Why are the changes needed?
It is a correctness bug: a literal `U+0000` is a valid replacement value,
but the in-band NUL deletion marker made `translate` drop the character instead
of replacing it. For example, `translate('A', 'A', char(0))` returned `''`
(deletion) instead of a single `U+0000` character.
### Does this PR introduce _any_ user-facing change?
Yes, in one edge case. When a character in `to` is a literal `U+0000`:
- Before: the matched character was deleted from the output.
- After: the matched character is replaced with `U+0000`.
Deletion semantics for the normal case (`to` shorter than `from`) are
unchanged. All other translations are unchanged.
### How was this patch tested?
Added regression coverage and ran the affected suites:
- `CollationSupportSuite.testStringTranslate` -- for each collation
(`UTF8_BINARY`, `UTF8_LCASE`, `UNICODE`, `UNICODE_CI`): literal `U+0000`
replacement is preserved (with a byte/length assertion), deletion with a
shorter `to`, mixed replacement + deletion, and first-occurrence-wins on a
duplicate `from` key. The test's dictionary-building helper was updated to the
empty-string marker to match production.
- `UTF8StringSuite.translate` -- migrated the existing deletion
dictionaries from the `"\0"` marker to `""`, and added a positive
literal-`U+0000` replacement case.
- `StringExpressionsSuite.translate` -- expression-level literal `U+0000`
replacement and mixed replacement/deletion cases.
All pass. I also micro-benchmarked the `translate` execution loop
(UTF8_BINARY / UTF8_LCASE) before and after; the change is performance-neutral
(`isEmpty()` is cheaper-or-equal to `"\0".equals(...)`), with differences
within run-to-run noise.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.8)
Closes #57052 from spyrospav/fix-SPARK-57747.
Authored-by: Spyros Pavlatos <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
(cherry picked from commit 715a437c34c4893d01b9838206b0b2d4d70acb88)
Signed-off-by: Wenchen Fan <[email protected]>
---
.../catalyst/util/CollationAwareUTF8String.java | 12 +++++----
.../org/apache/spark/unsafe/types/UTF8String.java | 5 ++--
.../spark/unsafe/types/CollationSupportSuite.java | 29 +++++++++++++++++++++-
.../apache/spark/unsafe/types/UTF8StringSuite.java | 14 +++++++----
.../catalyst/expressions/stringExpressions.scala | 10 +++++---
.../expressions/StringExpressionsSuite.scala | 5 ++++
6 files changed, 58 insertions(+), 17 deletions(-)
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 28c5a9f8473d..0da7af3ca640 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
@@ -1137,11 +1137,11 @@ public class CollationAwareUTF8String {
if (translated == null) {
// Append the original code point if no translation is found.
sb.appendCodePoint(codePoint);
- } else if (!"\0".equals(translated)) {
- // Append the translated code point if the translation is not the null
character.
+ } else if (!translated.isEmpty()) {
+ // Append the translation if it is not the deletion marker (empty
string).
sb.append(translated);
}
- // Skip the code point if it maps to the null character.
+ // Skip the code point if it maps to the empty string.
}
// Append the last code point if it was buffered.
if (codePointBuffer != -1) sb.appendCodePoint(codePointBuffer);
@@ -1204,8 +1204,10 @@ public class CollationAwareUTF8String {
++charIndex;
} else {
// We have found at least one match. Append the match of longest match
length to the output.
- if (!"\0".equals(dict.get(longestMatch))) {
- sb.append(dict.get(longestMatch));
+ // An empty mapping is the deletion marker, so append nothing in that
case.
+ String longestMatchTranslation = dict.get(longestMatch);
+ if (!longestMatchTranslation.isEmpty()) {
+ sb.append(longestMatchTranslation);
}
// Skip as many characters as the longest match.
charIndex += longestMatchLen;
diff --git
a/common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java
b/common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java
index 49429413904c..03c42785edb2 100644
--- a/common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java
+++ b/common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java
@@ -1724,9 +1724,10 @@ public final class UTF8String implements
Comparable<UTF8String>, Externalizable,
charCount = Character.charCount(codePoint);
String subStr = srcStr.substring(k, k + charCount);
String translated = dict.get(subStr);
- if (null == translated) {
+ if (translated == null) {
+ // No mapping for this character: keep the original.
sb.append(subStr);
- } else if (!"\0".equals(translated)) {
+ } else if (!translated.isEmpty()) {
sb.append(translated);
}
}
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 cd5b0f0ff962..9e7cf3893185 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
@@ -3896,13 +3896,40 @@ public class CollationSupportSuite {
assertStringTranslate("πΈ", "π", "x", UTF8_LCASE, "πΈ");
assertStringTranslate("πΈ", "π", "x", UNICODE, "πΈ");
assertStringTranslate("πΈ", "π", "x", UNICODE_CI, "x");
+ // Literal U+0000 in `to` is preserved as a one-character replacement, not
a deletion.
+ assertStringTranslate("A", "A", "\u0000", UTF8_BINARY, "\u0000");
+ assertStringTranslate("A", "A", "\u0000", UTF8_LCASE, "\u0000");
+ assertStringTranslate("A", "A", "\u0000", UNICODE, "\u0000");
+ assertStringTranslate("A", "A", "\u0000", UNICODE_CI, "\u0000");
+ // Deletion still applies when `to` is shorter than `from`.
+ assertStringTranslate("ABC", "BC", "X", UTF8_BINARY, "AX");
+ assertStringTranslate("ABC", "BC", "X", UTF8_LCASE, "AX");
+ assertStringTranslate("ABC", "BC", "X", UNICODE, "AX");
+ assertStringTranslate("ABC", "BC", "X", UNICODE_CI, "AX");
+ // Mixed literal U+0000 replacement and deletion within a single call:
+ // A -> U+0000, B -> X, and C, D are deleted (`to` is shorter than `from`).
+ assertStringTranslate("ABCD", "ABCD", "\u0000" + "X", UTF8_BINARY,
"\u0000" + "X");
+ assertStringTranslate("ABCD", "ABCD", "\u0000" + "X", UTF8_LCASE, "\u0000"
+ "X");
+ assertStringTranslate("ABCD", "ABCD", "\u0000" + "X", UNICODE, "\u0000" +
"X");
+ assertStringTranslate("ABCD", "ABCD", "\u0000" + "X", UNICODE_CI, "\u0000"
+ "X");
+ // Duplicate key in `from`: the first mapping wins over a later would-be
deletion.
+ assertStringTranslate("AB", "AA", "X", UTF8_BINARY, "XB");
+ assertStringTranslate("AB", "AA", "X", UTF8_LCASE, "XB");
+ assertStringTranslate("AB", "AA", "X", UNICODE, "XB");
+ assertStringTranslate("AB", "AA", "X", UNICODE_CI, "XB");
+ // Byte-level check: a literal U+0000 replacement yields exactly one code
point / one byte.
+ UTF8String nulResult = CollationSupport.StringTranslate.exec(
+ UTF8String.fromString("A"), buildDict("A", "\u0000"),
+ CollationFactory.collationNameToId(UTF8_BINARY));
+ assertEquals(1, nulResult.numChars());
+ assertEquals(1, nulResult.numBytes());
}
private Map<String, String> buildDict(String matching, String replace) {
Map<String, String> dict = new HashMap<>();
int i = 0, j = 0;
while (i < matching.length()) {
- String rep = "\u0000";
+ String rep = "";
if (j < replace.length()) {
int repCharCount = Character.charCount(replace.codePointAt(j));
rep = replace.substring(j, j + repCharCount);
diff --git
a/common/unsafe/src/test/java/org/apache/spark/unsafe/types/UTF8StringSuite.java
b/common/unsafe/src/test/java/org/apache/spark/unsafe/types/UTF8StringSuite.java
index 13d6c30cd256..420e49d0a26e 100644
---
a/common/unsafe/src/test/java/org/apache/spark/unsafe/types/UTF8StringSuite.java
+++
b/common/unsafe/src/test/java/org/apache/spark/unsafe/types/UTF8StringSuite.java
@@ -586,7 +586,7 @@ public class UTF8StringSuite {
"r", "1",
"n", "2",
"l", "3",
- "t", "\0"
+ "t", ""
)));
assertEquals(
fromString("translate"),
@@ -594,10 +594,10 @@ public class UTF8StringSuite {
assertEquals(
fromString("asae"),
fromString("translate").translate(Map.of(
- "r", "\0",
- "n", "\0",
- "l", "\0",
- "t", "\0"
+ "r", "",
+ "n", "",
+ "l", "",
+ "t", ""
)));
assertEquals(
fromString("aaδΈb"),
@@ -605,6 +605,10 @@ public class UTF8StringSuite {
"θ±", "a",
"η", "b"
)));
+ // A literal U+0000 replacement value is preserved (not treated as
deletion).
+ UTF8String withNul = fromString("abc").translate(Map.of("b", "\0"));
+ assertEquals(3, withNul.numChars());
+ assertEquals(fromString("a\0c"), withNul);
}
@Test
diff --git
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
index 71f30ca49d86..e41562f6f1d8 100755
---
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
+++
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala
@@ -1129,8 +1129,10 @@ object StringTranslate {
* Build a translation dictionary from Strings. This method assumes that the
input strings are
* already valid. The result dictionary maps each character in `matching` to
the corresponding
* character in `replace`. If `replace` is shorter than `matching`, the
extra characters in
- * `matching` will be mapped to null terminator, which causes characters to
get deleted during
- * translation. If `replace` is longer than `matching`, the extra characters
will be ignored.
+ * `matching` will be mapped to the empty string, which causes those
characters to get deleted
+ * during translation. If `replace` is longer than `matching`, the extra
characters will be
+ * ignored. Note that the empty string is used as the deletion marker so
that a literal `U+0000`
+ * in `replace` is preserved as a one-character replacement rather than
triggering deletion.
*/
private def buildDict(matching: String, replace: String): JMap[String,
String] = {
val dict = new HashMap[String, String]()
@@ -1144,12 +1146,12 @@ object StringTranslate {
j += repCharCount
repStr
} else {
- "\u0000"
+ "" // deletion marker
}
val matchCharCount = Character.charCount(matching.codePointAt(i))
val matchStr = matching.substring(i, i + matchCharCount)
- if (null == dict.get(matchStr)) {
+ if (!dict.containsKey(matchStr)) {
dict.put(matchStr, rep)
}
i += matchCharCount
diff --git
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/StringExpressionsSuite.scala
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/StringExpressionsSuite.scala
index 5559972cd795..711b5edd72ad 100644
---
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/StringExpressionsSuite.scala
+++
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/StringExpressionsSuite.scala
@@ -866,6 +866,11 @@ class StringExpressionsSuite extends SparkFunSuite with
ExpressionEvalHelper {
StringTranslate(Literal("translate"), Literal("rnlt"), Literal("123")),
"1a2s3ae")
checkEvaluation(StringTranslate(Literal("translate"), Literal(""),
Literal("123")), "translate")
checkEvaluation(StringTranslate(Literal("translate"), Literal("rnlt"),
Literal("")), "asae")
+ // A literal U+0000 in `to` is preserved as a one-character replacement,
not deletion.
+ checkEvaluation(StringTranslate(Literal("A"), Literal("A"),
Literal("\u0000")), "\u0000")
+ // Mixed literal U+0000 replacement and deletion: A -> U+0000, B -> X, C
and D deleted.
+ checkEvaluation(
+ StringTranslate(Literal("ABCD"), Literal("ABCD"), Literal("\u0000" +
"X")), "\u0000" + "X")
// test for multiple mapping
checkEvaluation(StringTranslate(Literal("abcd"), Literal("aba"),
Literal("123")), "12cd")
checkEvaluation(StringTranslate(Literal("abcd"), Literal("aba"),
Literal("12")), "12cd")
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
