[SPARK-57747][SQL] Fix translate() to distinguish deletion from literal U+0000 replacement#57052
Open
spyrospav wants to merge 1 commit into
Open
[SPARK-57747][SQL] Fix translate() to distinguish deletion from literal U+0000 replacement#57052spyrospav wants to merge 1 commit into
spyrospav wants to merge 1 commit into
Conversation
…al U+0000 replacement `translate(input, from, to)` both replaces matched 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 and checking `"\0".equals(...)` in the execution loop. That collided with a valid replacement value: translating a character to a literal `U+0000` produced deletion instead of a one-character NUL replacement. Switch the in-band deletion marker to the empty string, which cannot be a valid one-character replacement, and interpret dictionary values as: - null -> no mapping, keep the original character - empty -> delete the character - else -> replace (including a literal `U+0000`) Applied consistently across the UTF8_BINARY, UTF8_LCASE, and ICU collation translate paths and dictionary construction in `StringTranslate.buildDict`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
translate(input, from, to)replaces characters and, whentois shorter thanfrom, 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 literalU+0000was 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:
nullmapping -> no mapping, keep the original character;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) andCollationAwareUTF8String.translate(ICU path).Why are the changes needed?
It is a correctness bug: a literal
U+0000is a valid replacement value, but the in-band NUL deletion marker madetranslatedrop the character instead of replacing it. For example,translate('A', 'A', char(0))returned''(deletion) instead of a singleU+0000character.Does this PR introduce any user-facing change?
Yes, in one edge case. When a character in
tois a literalU+0000:U+0000.Deletion semantics for the normal case (
toshorter thanfrom) 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): literalU+0000replacement is preserved (with a byte/length assertion), deletion with a shorterto, mixed replacement + deletion, and first-occurrence-wins on a duplicatefromkey. 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+0000replacement case.StringExpressionsSuite.translate-- expression-level literalU+0000replacement and mixed replacement/deletion cases.All pass. I also micro-benchmarked the
translateexecution 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)