klsince commented on a change in pull request #7294:
URL: https://github.com/apache/pinot/pull/7294#discussion_r688166243



##########
File path: 
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/indexsegment/immutable/ImmutableSegmentLoader.java
##########
@@ -169,7 +158,20 @@ public static ImmutableSegment load(File indexDir, 
IndexLoadingConfig indexLoadi
 
     ImmutableSegmentImpl segment =
         new ImmutableSegmentImpl(actualSegmentDirectory, segmentMetadata, 
indexContainerMap, starTreeIndexContainer);
-    LOGGER.info("Successfully loaded segment {} with config: {}", segmentName, 
segmentDirectoryLoaderConfigs);
+    LOGGER.info("Successfully loaded segment {} with config: {}", segmentName, 
segDirConfigs);
     return segment;
   }
+
+  // Pre-process the segment on local using local SegmentDirectory.
+  // Please note that this step may modify the segment metadata.

Review comment:
       ack

##########
File path: 
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/SegmentPreProcessor.java
##########
@@ -104,77 +105,11 @@ public void process()
         LOGGER.warn("Skip creating default columns for segment: {} without 
schema", _segmentMetadata.getName());
       }
 
-      // Create column inverted indices according to the index config.
-      InvertedIndexHandler invertedIndexHandler =
-          new InvertedIndexHandler(_indexDir, _segmentMetadata, 
_indexLoadingConfig, segmentWriter);
-      invertedIndexHandler.createInvertedIndices();
-
-      // Create column range indices according to the index config.
-      RangeIndexHandler rangeIndexHandler =
-          new RangeIndexHandler(_indexDir, _segmentMetadata, 
_indexLoadingConfig, segmentWriter);
-      rangeIndexHandler.createRangeIndices();
-
-      // Create text indices according to the index config.
-      Set<String> textIndexColumns = _indexLoadingConfig.getTextIndexColumns();
-      if (!textIndexColumns.isEmpty()) {
-        TextIndexHandler textIndexHandler =
-            new TextIndexHandler(_indexDir, _segmentMetadata, 
textIndexColumns, segmentWriter);
-        textIndexHandler.createTextIndexesOnSegmentLoad();
-      }
-
-      Set<String> fstIndexColumns = _indexLoadingConfig.getFSTIndexColumns();
-      if (!fstIndexColumns.isEmpty()) {
-        LuceneFSTIndexHandler luceneFSTIndexHandler =
-            new LuceneFSTIndexHandler(_indexDir, _segmentMetadata, 
fstIndexColumns, segmentWriter);
-        luceneFSTIndexHandler.createFSTIndexesOnSegmentLoad();
-      }
+      createMissingIndices(segmentWriter);

Review comment:
       I kinda feel it natural to group them into one sub method, to make the 
main flow clear here. no strong preference tho.




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