siddharthteotia commented on a change in pull request #7918:
URL: https://github.com/apache/pinot/pull/7918#discussion_r774150730



##########
File path: 
pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java
##########
@@ -210,14 +212,15 @@ public String getSegmentMetadata(
               columnLength = storedDataType.size();
             } else if (columnMetadata.hasDictionary()) {
               // For type of variable width (String, Bytes), if it's stored 
using dictionary encoding, set the
-              // columnLength as the max
-              // length in dictionary.
+              // columnLength as the max length in dictionary.
               columnLength = columnMetadata.getColumnMaxLength();
             } else if (storedDataType == DataType.STRING || storedDataType == 
DataType.BYTES) {
               // For type of variable width (String, Bytes), if it's stored 
using raw bytes, set the columnLength as
-              // the length
-              // of the max value.
-              columnLength = ((String) 
columnMetadata.getMaxValue()).getBytes(StandardCharsets.UTF_8).length;
+              // the length of the max value.
+              ByteArray maxValueByteArray =
+                  storedDataType == DataType.BYTES ? ((ByteArray) 
columnMetadata.getMaxValue())
+                      : BytesUtils.toByteArray((String) 
columnMetadata.getMaxValue());
+              columnLength = maxValueByteArray.length();

Review comment:
       Discussed offline with @mqliang 
   
   As suggested in the previous comment, let's not fix the BYTES problem in 
this PR. We can investigate and fix that separately. The url encoding for (with 
and without *) and that corner case problem for a particular problem should be 
fixed (which @mqliang  found out is due to min/max value being null and server 
throwing NPE). 
   Let's make sure to add tests for with and without encoded query string -- 
everything after columns= can be encoded. I think in this PR, controller is 
re-encoding. We should remove that




-- 
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: commits-unsubscr...@pinot.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to