[FLINK-40350][table-runtime] LPAD/RPAD split supplementary-plane characters into unpaired surrogates by SEPURI-SAI-KRISHNA · Pull Request #29004 · apache/flink

SEPURI-SAI-KRISHNA · 2026-08-21T16:25:59Z

What is the purpose of the change

LPAD and RPAD measure length in UTF-16 code units instead of characters. When the requested length falls in the middle of a supplementary-plane character, the function splits the surrogate pair and returns a string containing an unpaired surrogate, which is not valid Unicode. The same code-unit arithmetic also makes the result shorter than requested whenever the base or the pad string contains such a character.

In the table below E stands for the single-character string U+1F600 GRINNING FACE, which UTF-16 encodes as the surrogate pair U+D83D U+DE00. It is written as E so that this description stays within the Basic Multilingual Plane.

Expression	Returned, as UTF-16 code units	Note
`LPAD(E, 1, 'x')`	`U+D83D`	unpaired high surrogate: the first half of `E`
`RPAD(E, 1, 'x')`	`U+D83D`	unpaired high surrogate: the first half of `E`
`RPAD('a', 4, E)`	`U+0061 U+D83D U+DE00 U+D83D`	pad split mid-pair, trailing unpaired high surrogate
`LPAD(E, 3, 'x')`	`U+0078 U+D83D U+DE00`	valid Unicode, but 2 characters instead of 3

This disagrees with the rest of the string functions, which already count characters rather than code units:

CHAR_LENGTH(E) is 1, because BinaryStringData#numChars advances by UTF-8 lead byte
SUBSTR(E, 1, 1) returns E intact, because SUBSTRING is generated against BinaryStringData

So SUBSTR(E, 1, 1) and LPAD(E, 1, 'x') are both "take the first 1 character of E" and give different answers, one of them not valid Unicode. The documentation agrees with the former:

Returns a new string from string1 left-padded with string2 to a length of integer characters.

Other engines return the character intact. Checked against Spark
(spark-sql, results shown as UTF-8 hex, F09F9880 is E):

Expression	Spark	Flink today	This PR
`lpad(E, 1, 'x')`	`F09F9880`	`U+D83D` alone	`F09F9880`
`rpad(E, 1, 'x')`	`F09F9880`	`U+D83D` alone	`F09F9880`
`rpad('a', 4, E)`	`61F09F9880F09F9880F09F9880`	trailing `U+D83D`	`61F09F9880F09F9880F09F9880`
`lpad(E, 3, 'x')`	`7878F09F9880`	2 characters	`7878F09F9880`

LPAD and RPAD are the only affected functions. SUBSTRING and OVERLAY are generated against BinaryStringData, and SqlFunctionUtils#subString and #overlay are not wired to any operator; initcap walks code units but rewrites only ASCII ranges and copies everything else through unchanged, so it never splits a pair.

The failure is silent: no exception is thrown and nothing is logged. The result is invalid UTF-16 and does not round-trip, so downstream string functions and comparisons then operate on a value the query never produced.

This is the same class of issue as FLINK-36267, which moved SPLIT to code-point iteration.

Brief change log

SqlFunctionUtils#lpad and #rpad measure both the base and the pad string in code points and slice only on character boundaries
The result is still built in a single exactly-sized char[]. The size is no longer len, since a supplementary-plane character occupies two chars, so it is computed from the kept part of the base plus the padding
Scanning of the base stops after len characters, so truncating a base far longer than the requested length no longer copies the whole base

Verifying this change

This change added tests and can be verified as follows:

Added lpadRpadTestCases to StringFunctionsITCase, which exercises the generated runtime code with field references rather than literals so the cases are not reduced at plan time. It covers a null base, a null pad and a null length, an empty pad, a negative length, truncation on a character boundary, padding with a supplementary-plane pad, a multi-character pad whose cycle is split mid-repeat, an empty base, and Basic Multilingual Plane cases that pin existing behaviour
Reverting only the SqlFunctionUtils change fails 36 of the 613 StringFunctionsITCase assertions with results such as expected: +I[<emoji>] but was: +I[?]; all 613 pass with the change applied. The flink-table-runtime suite is 1813/1813
A randomized differential check over 1M input combinations (2M lpad/rpad comparisons) agrees exactly with a straightforward code-point reference implementation, and every result holds exactly the requested number of code points with no unpaired surrogate
A further 500k Basic-Multilingual-Plane-only combinations agree exactly with the current implementation, so nothing moves for input without supplementary characters

Does this pull request potentially affect one of the following parts:

Dependencies (does it add or upgrade a dependency): no
The public API, i.e., is any changed class annotated with @Public(Evolving): no. SqlFunctionUtils is an internal runtime class, though the observable result of the LPAD and RPAD SQL functions changes for supplementary-plane input, which is the point of the fix
The serializers: no
The runtime per-record code paths (performance sensitive): yes. Both functions are per-record string functions, and the change is a small improvement. Best-of-5 over 2M calls each, against the current implementation: lpad("order-id-99213", 40, " ") 148.5 ns/op before and 127.9 ns/op after; lpad(<100k-char base>, 10, " ") 411.5 ms before and 0.6 ms after, since the current code calls toCharArray() on the whole base before discarding almost all of it
Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: no
The S3 file system connector: no

Documentation

Does this pull request introduce a new feature? no. It makes the implementation match the documented behaviour, so no documentation change is needed

Was generative AI tooling used to co-author this PR?

Yes (please specify the tool below)

Generated-by: Claude Code (Opus 5)

SqlFunctionUtils#lpad and #rpad allocated char[len] and indexed the input with String#length() and charAt, both of which count UTF-16 code units. A length falling inside a supplementary-plane character therefore split the surrogate pair and returned invalid Unicode, and a base or pad holding such a character produced a result with fewer characters than requested: LPAD(E, 1, 'x') -> lone high surrogate RPAD(E, 1, 'x') -> lone high surrogate RPAD('a', 4, E) -> pad split mid-pair, trailing lone high surrogate LPAD(E, 3, 'x') -> 2 characters instead of 3 where E stands for U+1F600 GRINNING FACE, encoded in UTF-16 as the surrogate pair D83D DE00. The documented contract is "a length of integer characters", and a supplementary-plane character is one character, so the input should be returned unchanged rather than halved. This also matches SPLIT, which moved to code-point iteration in FLINK-36267. Measure both base and pad in code points and slice on character boundaries, building the result with a StringBuilder. Padding is shared by both functions in appendPadding, which repeats the pad string cyclically and stops mid-string only on a character boundary. Scanning of the base stops after len characters, so truncating a base far longer than the requested length keeps the complexity of the previous implementation. Behaviour for Basic Multilingual Plane input is unchanged. Generated-by: Claude Code (Opus 5)

flinkbot · 2026-08-21T16:44:12Z

CI report:

168f137 Azure: SUCCESS

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot run azure re-run the last Azure build

… comments Compute the result size up front rather than assuming len chars, so the exact-size char[] of the original implementation is kept instead of a StringBuilder, and cut the helper Javadoc to one line each. Also cover null base, null pad and null length for LPAD and RPAD, which the test set was missing. Generated-by: Claude Code (Opus 5)

SEPURI-SAI-KRISHNA · 2026-08-23T01:06:00Z

Thanks for the review, all three are addressed in the new commit.

StringBuilder is gone; the size of the char[] is computed up front rather
than assumed to be len, and it now measures slightly faster than the current
implementation on the common case and much faster on truncation. Numbers are in
the inline reply. Comments are cut back to one line per helper.

I have also added null cases, which the test set was missing: null base, null
pad, a null INT length, plus empty pad and negative length, for both LPAD and
RPAD.

While re-checking I also confirmed this is inconsistent with the neighbouring
functions rather than just with the documentation: CHAR_LENGTH(E) is 1 and
SUBSTR(E, 1, 1) returns E intact, both via BinaryStringData, while
LPAD(E, 1, 'x') returns half of the surrogate pair. LPAD and RPAD are the only
two affected: SUBSTRING, LEFT, RIGHT and OVERLAY are generated against
BinaryStringData, SqlFunctionUtils#subString and #overlay are not wired to
any operator, and initcap rewrites only ASCII ranges so it never splits a
pair. I have added that to the PR description.

Both sql_functions.yml and sql_functions_zh.yml already describe the length
as characters, so no documentation change is needed here.

Verification on the new revision: reverting only the SqlFunctionUtils change
fails 36 of the StringFunctionsITCase assertions and passes all 613 with it
applied; ScalarFunctionsTest 99/99; the full flink-table-runtime suite
1813/1813; 1M randomized input combinations (2M lpad/rpad comparisons) match a
code-point reference implementation exactly, and 500k BMP-only combinations
match the current implementation exactly.

snuyanzin · 2026-08-23T05:05:11Z

+        int baseEnd = endOfCodePoints(base, len);
+        int padChars = padLength(pad, len - base.codePointCount(0, baseEnd));
+        char[] data = new char[padChars + baseEnd];


Suggested change

int baseEnd = endOfCodePoints(base, len);

int padChars = padLength(pad, len - base.codePointCount(0, baseEnd));

char[] data = new char[padChars + baseEnd];

final int baseEnd = endOfCodePoints(base, len);

final int padChars = padLength(pad, len - base.codePointCount(0, baseEnd));

final char[] data = new char[padChars + baseEnd];

Done. Two optional follow-ups here, want either?

base.codePointCount(0, baseEnd) re-walks what endOfCodePoints just walked; folding it in means inlining the loop in both methods.

when padChars == 0, base.substring(0, baseEnd) skips a copy.

snuyanzin · 2026-08-23T05:17:14Z

+    private static void writePad(char[] data, int pos, String pad, int chars) {
+        int end = pos + chars;
+        while (end - pos >= pad.length()) {
+            pad.getChars(0, pad.length(), data, pos);


why do we need to write on every iteration?

this is not true
why do we still have

System.arraycopy(data, pos, data, pos + written, next);

inside a loop?

You are right that a loop is still there, but the write is no longer per pad cycle: each pass doubles the region already written, so it is log2 rather than linear. For chars = 1000000, padLen = 2 that is 20 writes instead of 500001.

The JDK has no loop-free fill for a multi-char pattern. Want Arrays.fill for the single-char pad case, which is the common one?

why can't we count amount of symbols to pad in a loop
and then separately execute only 1 pad operation?

So i tried it: pad.repeat(n) then one getChars allocates the padding as a String first, so it measures slower, 309 vs 142 ns at 1000 chars.

String.repeat is itself this loop: JDK 17 does System.arraycopy with copied <<= 1 for a multi-char string, and Arrays.fill when the string is 1 char.

The second half looks worth taking: Arrays.fill for padLen == 1 is faster up to ~1000 chars (4 vs 18 ns at 8 chars, 11 vs 39 at 128). Add that and keep the doubling for longer pads?

snuyanzin

@SEPURI-SAI-KRISHNA please do NOT flood with your AI in comments.
What is the reason to write a poem here?

Also PR's tittle Measure LPAD/RPAD length in characters says nothing about the actual issue

…nal locals Fill the padding by doubling an initial copy with System.arraycopy instead of writing the pad on every iteration. Drop the LPAD/RPAD cases from ScalarFunctionsTest; they duplicate the ones in StringFunctionsITCase. Generated-by: Claude Code (Opus 5)

snuyanzin reviewed Aug 22, 2026

View reviewed changes