[GitHub] [pinot] siddharthteotia commented on a change in pull request #7918: Fix column metadata getMaxValue NPE bug and expose maxNumMultiValues

[GitBox <gi...@apache.org>]

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



##########
File path: pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java
##########
@@ -210,21 +211,22 @@ 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.
+              String maxString = (String) columnMetadata.getMaxValue();
+              columnLength = maxString == null ? 0 : maxString.getBytes(StandardCharsets.UTF_8).length;
             } else {
               // For type of STRUCT, MAP, LIST, set the columnLength as DEFAULT_MAX_LENGTH (512).
               columnLength = FieldSpec.DEFAULT_MAX_LENGTH;
             }
             int columnCardinality = segmentMetadata.getColumnMetadataMap().get(column).getCardinality();
+            int maxNumMultiValues = segmentMetadata.getColumnMetadataMap().get(column).getMaxNumberOfMultiValues();
             columnLengthMap.merge(column, (double) columnLength, Double::sum);
             columnCardinalityMap.merge(column, (double) columnCardinality, Double::sum);
+            maxNumMultiValuesMap.merge(column, (double) maxNumMultiValues, Double::sum);

Review comment:
       Can we avoid populating this map for single valued column since maxNumMultiValues is only applicable to MV columns ? The other option is to make sure it is either 0 or -1 for SV columns




-- 
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-unsubscribe@pinot.apache.org

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



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