xiangfu0 commented on code in PR #11669:
URL: https://github.com/apache/pinot/pull/11669#discussion_r1336277208


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/fwd/MultiValueVarByteRawIndexCreator.java:
##########
@@ -77,13 +76,17 @@ public MultiValueVarByteRawIndexCreator(File baseIndexDir, 
ChunkCompressionType
     //we will prepend the actual content with numElements and length array 
containing length of each element
     int totalMaxLength = getTotalRowStorageBytes(maxNumberOfElements, 
maxRowLengthInBytes);
 
-    File file = new File(baseIndexDir,
-        column + Indexes.RAW_MV_FORWARD_INDEX_FILE_EXTENSION);
+    File file = new File(baseIndexDir, column + 
Indexes.RAW_MV_FORWARD_INDEX_FILE_EXTENSION);
     int numDocsPerChunk = Math.max(
-        TARGET_MAX_CHUNK_SIZE / (totalMaxLength + 
VarByteChunkSVForwardIndexWriter.CHUNK_HEADER_ENTRY_ROW_OFFSET_SIZE),
+        TARGET_MAX_CHUNK_SIZE / (totalMaxLength + 
VarByteChunkForwardIndexWriter.CHUNK_HEADER_ENTRY_ROW_OFFSET_SIZE),
         1);
-    _indexWriter = new VarByteChunkSVForwardIndexWriter(file, compressionType, 
totalDocs, numDocsPerChunk,
-        totalMaxLength, writerVersion);
+    // TODO: Support V4 MV reader
+    // Currently fall back to V2 for backward compatible
+    if (writerVersion >= VarByteChunkForwardIndexWriterV4.VERSION) {

Review Comment:
   just use `writerVersion == VarByteChunkForwardIndexWriterV4.VERSION` to be 
explicit?



##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/columnminmaxvalue/ColumnMinMaxValueGenerator.java:
##########
@@ -188,194 +187,162 @@ private void addColumnMinMaxValueForColumn(String 
columnName)
     } else {
       // setting min/max for non-dictionary columns.
       int numDocs = columnMetadata.getTotalDocs();
-      boolean isSingleValueField = 
_segmentMetadata.getSchema().getFieldSpecFor(columnName).isSingleValueField();
-      PinotDataBuffer forwardBuffer = _segmentWriter.getIndexFor(columnName, 
StandardIndexes.forward());
-      switch (dataType) {
-        case INT: {
-          int min = Integer.MAX_VALUE;
-          int max = Integer.MIN_VALUE;
-          if (isSingleValueField) {
-            try (FixedByteChunkSVForwardIndexReader rawIndexReader = new 
FixedByteChunkSVForwardIndexReader(
-                forwardBuffer, DataType.INT); ChunkReaderContext readerContext 
= rawIndexReader.createContext()) {
-                for (int docId = 0; docId < numDocs; docId++) {
-                  int value = rawIndexReader.getInt(docId, readerContext);
+      PinotDataBuffer rawIndexBuffer = _segmentWriter.getIndexFor(columnName, 
StandardIndexes.forward());
+      boolean isSingleValue = 
_segmentMetadata.getSchema().getFieldSpecFor(columnName).isSingleValueField();
+      try (
+          ForwardIndexReader rawIndexReader = 
ForwardIndexReaderFactory.createRawIndexReader(rawIndexBuffer, storedType,
+              isSingleValue); ForwardIndexReaderContext readerContext = 
rawIndexReader.createContext()) {
+        switch (storedType) {
+          case INT: {
+            int min = Integer.MAX_VALUE;
+            int max = Integer.MIN_VALUE;
+            if (isSingleValue) {
+              for (int docId = 0; docId < numDocs; docId++) {
+                int value = rawIndexReader.getInt(docId, readerContext);
+                min = Math.min(min, value);
+                max = Math.max(max, value);
+              }
+            } else {
+              for (int docId = 0; docId < numDocs; docId++) {
+                int[] values = rawIndexReader.getIntMV(docId, readerContext);
+                for (int value : values) {
                   min = Math.min(min, value);
                   max = Math.max(max, value);
                 }
+              }
             }
-          } else {
-            try (FixedByteChunkMVForwardIndexReader rawIndexReader = new 
FixedByteChunkMVForwardIndexReader(
-                forwardBuffer, DataType.INT); ChunkReaderContext readerContext 
= rawIndexReader.createContext()) {
-                for (int docId = 0; docId < numDocs; docId++) {
-                  int[] value = rawIndexReader.getIntMV(docId, readerContext);
-                  for (int i = 0; i < value.length; i++) {
-                    min = Math.min(min, value[i]);
-                    max = Math.max(max, value[i]);
-                  }
-                }
-            }
+            
SegmentColumnarIndexCreator.addColumnMinMaxValueInfo(_segmentProperties, 
columnName, String.valueOf(min),

Review Comment:
   This is common and can be moved out of switch case statement?



-- 
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]

Reply via email to