snuyanzin commented on code in PR #29004:
URL: https://github.com/apache/flink/pull/29004#discussion_r3923318875
##########
flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/functions/SqlFunctionUtils.java:
##########
@@ -345,31 +345,60 @@ public static String rpad(String base, int len, String
pad) {
return "";
}
- char[] data = new char[len];
- char[] baseChars = base.toCharArray();
- char[] padChars = pad.toCharArray();
-
- int pos = 0;
-
- // copy the base
- while (pos < base.length() && pos < len) {
- data[pos] = baseChars[pos];
- pos += 1;
+ final int length = base.length();
+ int baseEnd = 0;
+ int baseCodePoints = 0;
+ while (baseCodePoints < len && baseEnd < length) {
+ baseEnd += Character.charCount(base.codePointAt(baseEnd));
+ baseCodePoints++;
}
- // copy the padding
- while (pos < len) {
- int i = 0;
- while (i < pad.length() && i < len - pos) {
- data[pos + i] = padChars[i];
- i += 1;
- }
- pos += pad.length();
+ final int padCount = len - baseCodePoints;
+ if (padCount == 0) {
+ // The base already holds len code points, so it only needs
truncating.
+ return base.substring(0, baseEnd);
}
+ final int padChars = padLength(pad, padCount);
+ final char[] data = new char[Math.addExact(baseEnd, padChars)];
+
+ base.getChars(0, baseEnd, data, 0);
+ writePad(data, baseEnd, pad, padChars);
+
return new String(data);
}
+ /** Number of chars taken by count code points of pad repeated cyclically.
*/
+ private static int padLength(String pad, int count) {
+ final int padLen = pad.length();
+ final int cycle = pad.codePointCount(0, padLen);
+ return Math.addExact(
+ Math.multiplyExact(count / cycle, padLen),
+ pad.offsetByCodePoints(0, count % cycle));
+ }
+
+ /** Writes chars characters into data at pos, repeating pad cyclically. */
+ private static void writePad(char[] data, int pos, String pad, int chars) {
+ final int padLen = pad.length();
+ if (padLen == 1) {
+ // A single char fills directly, which is faster than the doubling
copy below.
+ Arrays.fill(data, pos, pos + chars, pad.charAt(0));
+ return;
+ }
+
+ // The pad is written once and then each pass doubles the region
already written,
+ // so the fill takes log2(chars) copies rather than one copy per pad
cycle.
Review Comment:
in fact it says nothing about the reason...
it just duplicates what could be read from the code.
The comment we need here: explanation why we need this code and why the case
here could not be handled with the code above with `Array.fill`
--
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]
