This is an automated email from the ASF dual-hosted git repository.
Jackie-Jiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new a7cfea0b824 Upgrade Commons Lang to 3.21.0 and discard trailing empty
split fields (#19747)
a7cfea0b824 is described below
commit a7cfea0b824a2c30a0faf1e0a80a42d720e1a12a
Author: Xiaotian (Jackie) Jiang <[email protected]>
AuthorDate: Sun Oct 4 21:23:35 2026 -0700
Upgrade Commons Lang to 3.21.0 and discard trailing empty split fields
(#19747)
---
.../common/function/scalar/StringFunctions.java | 72 +++++------------
.../function/scalar/StringFunctionsTest.java | 89 ++++++++++++++++------
pom.xml | 2 +-
3 files changed, 86 insertions(+), 77 deletions(-)
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java
b/pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java
index da33990b79b..bbb73a54e64 100644
---
a/pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java
+++
b/pinot-common/src/main/java/org/apache/pinot/common/function/scalar/StringFunctions.java
@@ -493,7 +493,9 @@ public class StringFunctions {
return suffixArr;
}
- /// TODO: Revisit if index should be one-based (both Presto and Postgres use
one-based index, which starts with 1)
+ /// Empty fields from leading, consecutive, and trailing delimiters are
discarded.
+ /// TODO: Revisit whether to use one-based indexes and preserve empty fields
from leading, consecutive, and trailing
+ /// delimiters, as Presto and Postgres do.
/// @param input the input String to be split into parts.
/// @param delimiter the specified delimiter to split the input string.
/// @param index we allow negative value for index which indicates the index
from the end.
@@ -517,8 +519,8 @@ public class StringFunctions {
while (start < len && input.startsWith(delimiter, start)) {
start += delimLen;
}
- // Guard against Integer.MIN_VALUE since negating it overflows (remains
negative)
- if (index == Integer.MIN_VALUE) {
+ // No fields remain if the input contains only delimiters. Negating
Integer.MIN_VALUE would overflow.
+ if (start == len || index == Integer.MIN_VALUE) {
return "null";
}
// optimization for negative index with single-char delimiter since common
case
@@ -532,7 +534,7 @@ public class StringFunctions {
if (adjustedIndex < 0) {
int totalFields = 0;
int pos = start;
- while (pos <= len) {
+ while (pos < len) {
totalFields++;
int end = input.indexOf(delimiter, pos);
if (end == -1) {
@@ -561,34 +563,23 @@ public class StringFunctions {
}
}
+ if (start == len) {
+ return "null";
+ }
int end = input.indexOf(delimiter, start);
return end == -1 ? input.substring(start) : input.substring(start, end);
}
private static String splitPartNegativeIdxSingleCharDelim(
String input, char delimiter, int index, int len, int start) {
- // input is empty or contains only delimiters
- if (start == len) {
- return index == 1 ? "" : "null";
- }
-
- // scan backwards and handle trailing delimiters
+ // Scan backwards past trailing delimiters without counting empty fields.
int end = len;
while (end > start && input.charAt(end - 1) == delimiter) {
end--;
}
- // handle trailing delimiters
- int resultIdx = index;
- if (end < len) {
- if (index == 1) {
- return "";
- }
- resultIdx--;
- }
-
int curEnd = end;
- for (int i = 1; i <= resultIdx; i++) {
+ for (int i = 1; i <= index; i++) {
// handle left out of bound index
if (curEnd <= start) {
return "null";
@@ -600,7 +591,7 @@ public class StringFunctions {
curStart--;
}
- if (i == resultIdx) {
+ if (i == index) {
return input.substring(curStart + 1, curEnd);
}
@@ -628,8 +619,8 @@ public class StringFunctions {
/// Avoids allocating the full String array by scanning the input directly.
///
/// Replicates the semantics of [StringUtils#splitByWholeSeparator(String,
String, int)]:
- /// leading separators are stripped, consecutive separators in the middle
are collapsed,
- /// and trailing separators produce one empty trailing token.
+ /// empty fields from leading, consecutive, and trailing separators are
discarded.
+ /// Once the limit is reached, the last field contains the unsplit
remainder, including any trailing separators.
///
/// @param input the input String to be split into parts.
/// @param delimiter the specified delimiter to split the input string.
@@ -674,8 +665,8 @@ public class StringFunctions {
}
/// Counts the number of fields produced by splitting input with the given
delimiter and limit,
- /// following splitByWholeSeparator semantics (leading seps stripped,
consecutive collapsed,
- /// trailing seps produce one empty field). Does not allocate any String
objects.
+ /// following splitByWholeSeparator semantics (empty fields discarded, final
field contains the unsplit remainder
+ /// when the limit is reached). Does not allocate any String objects.
private static int countFieldsLimited(String input, String delimiter, int
effectiveLimit,
int inputLen, int delimLen) {
int pos = 0;
@@ -683,11 +674,6 @@ public class StringFunctions {
while (pos < inputLen && input.startsWith(delimiter, pos)) {
pos += delimLen;
}
- if (pos >= inputLen) {
- // Entire string is separators — produces a single empty trailing token
- return 1;
- }
-
int totalFields = 0;
while (pos < inputLen) {
totalFields++;
@@ -706,18 +692,13 @@ public class StringFunctions {
while (pos < inputLen && input.startsWith(delimiter, pos)) {
pos += delimLen;
}
- // If we've consumed to end after delimiters, there's a trailing empty
field
- if (pos >= inputLen) {
- totalFields++;
- break;
- }
}
return totalFields;
}
/// Extracts the field at the given positive index by scanning forward
through the input.
- /// Follows splitByWholeSeparator semantics: leading separators stripped,
consecutive collapsed,
- /// trailing separators produce one empty token. With limit, the last field
gets the remainder.
+ /// Follows splitByWholeSeparator semantics: empty fields discarded.
+ /// With limit, the last field gets the unsplit remainder, including any
trailing separators.
private static String splitPartLimitedForward(String input, String
delimiter, int effectiveLimit, int index,
int inputLen, int delimLen) {
int pos = 0;
@@ -725,13 +706,8 @@ public class StringFunctions {
while (pos < inputLen && input.startsWith(delimiter, pos)) {
pos += delimLen;
}
- if (pos >= inputLen) {
- // Entire string is separators — single empty trailing token at index 0
- return index == 0 ? "" : "null";
- }
-
int fieldCount = 0;
- while (pos <= inputLen) {
+ while (pos < inputLen) {
// Check if this is the last field due to limit
if (fieldCount + 1 >= effectiveLimit) {
// Limit reached — remainder from pos to end is the last field
@@ -745,8 +721,6 @@ public class StringFunctions {
if (fieldCount == index) {
return input.substring(pos);
}
- // Check if input ends with delimiter characters that were already
consumed
- // (this case is handled by the trailing-delimiter logic below)
return "null";
}
@@ -761,14 +735,6 @@ public class StringFunctions {
pos += delimLen;
}
fieldCount++;
-
- // If we've consumed to the end after delimiters, there's a trailing
empty field
- if (pos >= inputLen) {
- if (fieldCount == index) {
- return "";
- }
- return "null";
- }
}
return "null";
}
diff --git
a/pinot-common/src/test/java/org/apache/pinot/common/function/scalar/StringFunctionsTest.java
b/pinot-common/src/test/java/org/apache/pinot/common/function/scalar/StringFunctionsTest.java
index 56bed3f22ad..ed6f3359df1 100644
---
a/pinot-common/src/test/java/org/apache/pinot/common/function/scalar/StringFunctionsTest.java
+++
b/pinot-common/src/test/java/org/apache/pinot/common/function/scalar/StringFunctionsTest.java
@@ -46,7 +46,7 @@ public class StringFunctionsTest {
{"org.apache.pinot.common.function", ".", 4, 5, "function",
"function"},
{"org.apache.pinot.common.function", ".", 5, 6, "null", "null"},
{"org.apache.pinot.common.function", ".", 3, 3, "common", "null"},
- {"+++++", "+", 0, 100, "", ""},
+ {"+++++", "+", 0, 100, "null", "null"},
{"+++++", "+", 1, 100, "null", "null"},
{"+++++org++apache++", "", 1, 100, "null", "null"},
{"+++++org++apache++", "", 0, 100, "+++++org++apache++",
"+++++org++apache++"},
@@ -70,7 +70,7 @@ public class StringFunctionsTest {
{"org.apache.pinot.common.function", ".", -1, 6, "function",
"function"},
{"org.apache.pinot.common.function", ".", -5, 6, "org", "org"},
{"org.apache.pinot.common.function", ".", -6, 6, "null", "null"},
- {"+++++", "+", -1, 100, "", ""},
+ {"+++++", "+", -1, 100, "null", "null"},
{"+++++", "+", -2, 100, "null", "null"},
// Empty delimiter: index=-1 returns input, other negative indices
return "null"
@@ -84,34 +84,34 @@ public class StringFunctionsTest {
{"abc", ".", -2, 100, "null", "null"},
// Input equals delimiter
- {".", ".", 0, 100, "", ""},
+ {".", ".", 0, 100, "null", "null"},
{".", ".", 1, 100, "null", "null"},
- {".", ".", -1, 100, "", ""},
+ {".", ".", -1, 100, "null", "null"},
// Trailing delimiters with content (single-char): exercises
splitPartNegativeIdxSingleCharDelim
- // trailing-delimiter handling (resultIdx decrement, empty trailing
field)
+ // without counting empty trailing fields
{"org++apache++", "+", 0, 100, "org", "org"},
{"org++apache++", "+", 1, 100, "apache", "apache"},
- {"org++apache++", "+", 2, 100, "", ""},
+ {"org++apache++", "+", 2, 100, "null", "null"},
{"org++apache++", "+", 3, 100, "null", "null"},
- {"org++apache++", "+", -1, 100, "", ""},
- {"org++apache++", "+", -2, 100, "apache", "apache"},
- {"org++apache++", "+", -3, 100, "org", "org"},
+ {"org++apache++", "+", -1, 100, "apache", "apache"},
+ {"org++apache++", "+", -2, 100, "org", "org"},
+ {"org++apache++", "+", -3, 100, "null", "null"},
{"org++apache++", "+", -4, 100, "null", "null"},
// Leading AND trailing delimiters (single-char): exercises backward
scan with
- // both leading delimiter skip and trailing delimiter adjustment
+ // both leading and trailing delimiters skipped
{"++org++apache++", "+", 0, 100, "org", "org"},
{"++org++apache++", "+", 1, 100, "apache", "apache"},
- {"++org++apache++", "+", -1, 100, "", ""},
- {"++org++apache++", "+", -2, 100, "apache", "apache"},
- {"++org++apache++", "+", -3, 100, "org", "org"},
+ {"++org++apache++", "+", -1, 100, "apache", "apache"},
+ {"++org++apache++", "+", -2, 100, "org", "org"},
+ {"++org++apache++", "+", -3, 100, "null", "null"},
{"++org++apache++", "+", -4, 100, "null", "null"},
// Single field surrounded by delimiters
{"++abc++", "+", 0, 100, "abc", "abc"},
- {"++abc++", "+", -1, 100, "", ""},
- {"++abc++", "+", -2, 100, "abc", "abc"},
+ {"++abc++", "+", -1, 100, "abc", "abc"},
+ {"++abc++", "+", -2, 100, "null", "null"},
{"++abc++", "+", -3, 100, "null", "null"},
// Multi-char delimiter: exercises forward scan and multi-char
negative index
@@ -136,15 +136,41 @@ public class StringFunctionsTest {
{"::::org::::apache", "::", -3, 100, "null", "null"},
// Multi-char delimiter with leading AND trailing delimiters:
exercises the
- // trailing empty field in the multi-char totalFields counting path
+ // multi-char totalFields counting path without empty trailing fields
{"::org::apache::", "::", 0, 100, "org", "org"},
{"::org::apache::", "::", 1, 100, "apache", "apache"},
- {"::org::apache::", "::", 2, 100, "", ""},
- {"::org::apache::", "::", -1, 100, "", ""},
- {"::org::apache::", "::", -2, 100, "apache", "apache"},
- {"::org::apache::", "::", -3, 100, "org", "org"},
+ {"::org::apache::", "::", 2, 100, "null", "null"},
+ {"::org::apache::", "::", -1, 100, "apache", "apache"},
+ {"::org::apache::", "::", -2, 100, "org", "org"},
+ {"::org::apache::", "::", -3, 100, "null", "null"},
{"::org::apache::", "::", -4, 100, "null", "null"},
+ // Trailing delimiters are discarded unless they belong to the unsplit
remainder at the limit.
+ {"a,b,", ",", 1, 1, "b", "null"},
+ {"a,b,", ",", -1, 1, "b", "a,b,"},
+ {"a,b,", ",", 1, 2, "b", "b,"},
+ {"a,b,", ",", -1, 2, "b", "b,"},
+ {"a,b,", ",", 2, 2, "null", "null"},
+ {"a,b,", ",", -2, 2, "a", "a"},
+ {"a,b,", ",", -1, 3, "b", "b"},
+ {"a,b,", ",", 2, 3, "null", "null"},
+ {",,a,,b,,", ",", -1, 0, "b", "b"},
+ {",,a,,b,,", ",", -1, -1, "b", "b"},
+ {",,a,,b,,", ",", -1, 2, "b", "b,,"},
+ {"::a::::b::::", "::", 1, 2, "b", "b::::"},
+ {"::a::::b::::", "::", -1, 2, "b", "b::::"},
+ {"::a::::b::::", "::", -1, 3, "b", "b"},
+ {"::a::::b::::", "::", -2, 3, "a", "a"},
+ {"::a::::b::::", "::", 2, 3, "null", "null"},
+ {",,,", ",", 0, 1, "null", "null"},
+ {",,,", ",", -1, 1, "null", "null"},
+ {"::::", "::", 0, 1, "null", "null"},
+ {"::::", "::", -1, 1, "null", "null"},
+ // A suffix that is only part of a multi-character delimiter is a
non-empty field.
+ {"aaaaa", "aa", 0, 100, "a", "a"},
+ {"aaaaa", "aa", -1, 100, "a", "a"},
+ {"ababa", "aba", -1, 100, "ba", "ba"},
+
// Empty input with non-empty delimiter
{"", ".", 0, 100, "null", "null"},
{"", ".", -1, 100, "null", "null"},
@@ -155,6 +181,7 @@ public class StringFunctionsTest {
// Integer.MIN_VALUE: negating it overflows (remains negative), guard
must return "null"
{"org.apache.pinot", ".", Integer.MIN_VALUE, 100, "null", "null"},
+ {"a,b,", ",", Integer.MAX_VALUE, 2, "null", "null"},
};
}
@@ -304,11 +331,21 @@ public class StringFunctionsTest {
assertEquals(StringFunctions.splitPart(input, delimiter, limit, index),
expectedTokenWithLimitCounts);
}
+ @Test
+ public void testSplitDiscardsEmptyFields() {
+ assertEquals(StringFunctions.split(",,a,,b,,", ","), new String[]{"a",
"b"});
+ assertEquals(StringFunctions.split("::a::::b::::", "::"), new
String[]{"a", "b"});
+ assertEquals(StringFunctions.split(",,,", ","), new String[0]);
+ assertEquals(StringFunctions.split("::::", "::"), new String[0]);
+ assertEquals(StringFunctions.split("a,b,", ",", 2), new String[]{"a",
"b,"});
+ assertEquals(StringFunctions.split("a,b,", ",", 3), new String[]{"a",
"b"});
+ }
+
@Test
public void testSplitPartRandomized() {
- String chars = "abcdefg.,:;+-_/";
- String[] delimiters = {".", ",", ":", "::", "++", "ab", "///"};
- Random random = new Random();
+ String chars = "abcdefg.,:;+-_/ \t";
+ String[] delimiters = {".", ",", ":", "::", "++", "ab", "///", "", " "};
+ Random random = new Random(0);
int numIterations = 10_000;
for (int iter = 0; iter < numIterations; iter++) {
@@ -326,6 +363,12 @@ public class StringFunctionsTest {
String actual = StringFunctions.splitPart(input, delimiter, index);
assertEquals(actual, expected,
String.format("Mismatch for input='%s', delimiter='%s', index=%d",
input, delimiter, index));
+
+ int limit = random.nextInt(8) - 2;
+ String expectedWithLimit = StringFunctions.splitPartArrayBased(
+ StringUtils.splitByWholeSeparator(input, delimiter, limit), index);
+ assertEquals(StringFunctions.splitPart(input, delimiter, limit, index),
expectedWithLimit,
+ String.format("Mismatch for input='%s', delimiter='%s', limit=%d,
index=%d", input, delimiter, limit, index));
}
}
diff --git a/pom.xml b/pom.xml
index a2b7e0adf73..3aa2cd22432 100644
--- a/pom.xml
+++ b/pom.xml
@@ -263,7 +263,7 @@
<flink.version>2.3.0</flink.version>
<!-- Apache Commons Libraries -->
- <commons-lang3.version>3.20.0</commons-lang3.version>
+ <commons-lang3.version>3.21.0</commons-lang3.version>
<commons-collections4.version>4.6.0</commons-collections4.version>
<commons-text.version>1.15.0</commons-text.version>
<commons-compress.version>1.28.0</commons-compress.version>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
