spyrospav opened a new pull request, #57052:
URL: https://github.com/apache/spark/pull/57052
### 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)
--
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]
