Jackie-Jiang commented on code in PR #11993:
URL: https://github.com/apache/pinot/pull/11993#discussion_r1406958490


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/ForwardIndexHandler.java:
##########
@@ -381,37 +368,64 @@ private boolean shouldDisableDictionary(String column, 
ColumnMetadata existingCo
     return true;
   }
 
-  private boolean shouldChangeCompressionType(String column, 
SegmentDirectory.Reader segmentReader)
+  private boolean shouldChangeRawCompressionType(String column, 
SegmentDirectory.Reader segmentReader)
       throws Exception {
-    ColumnMetadata existingColMetadata = 
_segmentDirectory.getSegmentMetadata().getColumnMetadataFor(column);
-
     // The compression type for an existing segment can only be determined by 
reading the forward index header.
+    ColumnMetadata existingColMetadata = 
_segmentDirectory.getSegmentMetadata().getColumnMetadataFor(column);
+    ChunkCompressionType existingCompressionType;
     try (ForwardIndexReader<?> fwdIndexReader = 
ForwardIndexType.read(segmentReader, existingColMetadata)) {
-      ChunkCompressionType existingCompressionType = 
fwdIndexReader.getCompressionType();
+      existingCompressionType = fwdIndexReader.getCompressionType();
       Preconditions.checkState(existingCompressionType != null,
           "Existing compressionType cannot be null for raw forward index 
column=" + column);
+    }
 
-      // Get the new compression type.
-      ChunkCompressionType newCompressionType = 
_fieldIndexConfigs.get(column).getConfig(StandardIndexes.forward())
-          .getChunkCompressionType();
+    // Get the new compression type.
+    ChunkCompressionType newCompressionType =
+        
_fieldIndexConfigs.get(column).getConfig(StandardIndexes.forward()).getChunkCompressionType();
 
-      // Note that default compression type (PASS_THROUGH for metric and LZ4 
for dimension) is not considered if the
-      // compressionType is not explicitly provided in tableConfig. This is to 
avoid incorrectly rewriting all the
-      // forward indexes during segmentReload when the default compressionType 
changes.
-      return newCompressionType != null && existingCompressionType != 
newCompressionType;
+    // Note that default compression type (PASS_THROUGH for metric and LZ4 for 
dimension) is not considered if the
+    // compressionType is not explicitly provided in tableConfig. This is to 
avoid incorrectly rewriting all the
+    // forward indexes during segmentReload when the default compressionType 
changes.
+    return newCompressionType != null && existingCompressionType != 
newCompressionType;
+  }
+
+  private boolean shouldChangeDictCompressionType(String column, 
SegmentDirectory.Reader segmentReader)
+      throws Exception {
+    // The compression type for an existing segment can only be determined by 
reading the forward index header.
+    ColumnMetadata existingColMetadata = 
_segmentDirectory.getSegmentMetadata().getColumnMetadataFor(column);
+    DictCompressionType existingCompressionType;
+    try (ForwardIndexReader<?> fwdIndexReader = 
ForwardIndexType.read(segmentReader, existingColMetadata)) {
+      existingCompressionType = fwdIndexReader.getDictCompressionType();
     }
+
+    // Get the new compression type.
+    DictCompressionType newCompressionType =
+        
_fieldIndexConfigs.get(column).getConfig(StandardIndexes.forward()).getDictCompressionType();
+    // MV_ENTRY_DICT can only be applied to multi-value columns.
+    if (newCompressionType == DictCompressionType.MV_ENTRY_DICT && 
existingColMetadata.isSingleValue()) {

Review Comment:
   Done



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