Jackie-Jiang commented on code in PR #11218:
URL: https://github.com/apache/pinot/pull/11218#discussion_r1278801877
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/SegmentColumnarIndexCreator.java:
##########
@@ -605,30 +615,30 @@ static boolean isValidPropertyValue(String value) {
@VisibleForTesting
static String getValidPropertyValue(String value, boolean isMax, DataType
dataType) {
String valueWithinLengthLimit = getValueWithinLengthLimit(value, isMax,
dataType);
- int length = valueWithinLengthLimit.length();
+ String escapedValue = StringEscapeUtils.escapeJava(valueWithinLengthLimit);
+ int length = escapedValue.length();
if (length > 0) {
// check and replace first character if it's a white space
- if (Character.isWhitespace(valueWithinLengthLimit.charAt(0))) {
- String unicodeValue = "\\u" + String.format("%04x", (int)
valueWithinLengthLimit.charAt(0));
- valueWithinLengthLimit = unicodeValue +
valueWithinLengthLimit.substring(1);
+ if (Character.isWhitespace(escapedValue.charAt(0))) {
Review Comment:
Thanks for taking time to do the research. Great finding that the value is
already escaped in the property file.
I tried explicitly escape the `","` by replacing it with `"\\,"`, but it
doesn't work if it is preceded by a backslash (I tried replacing `"\\,"` with
`"\\\\\\,"` which works in memory, but once the file is saved and read again,
it breaks).
To solve both special cases (leading/trailing space and comma), one
workaround we can take is to use a preserved character to replace them. Here we
can use `"\0"` because it is preserved as the padding character for string
values, and string values cannot include it (See
`StringUtil.sanitizeStringValue()` where we explicitly remove it).
Implemented the idea in #11223
--
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]
