gortiz commented on code in PR #10687:
URL: https://github.com/apache/pinot/pull/10687#discussion_r1176663536


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/indexsegment/mutable/MutableSegmentImpl.java:
##########
@@ -245,32 +239,32 @@ public boolean isMutableSegment() {
     _logger =
         LoggerFactory.getLogger(MutableSegmentImpl.class.getName() + "_" + 
_segmentName + "_" + config.getStreamName());
 
-    // Here we create a copy of no dictionary columns because we are going to 
mutate the set some lines bellow
-    Set<String> noDictionaryColumns = new 
HashSet<>(config.getNoDictionaryColumns());
-    Set<String> invertedIndexColumns = config.getInvertedIndexColumns();
-    Set<String> textIndexColumns = config.getTextIndexColumns();
-    Set<String> fstIndexColumns = config.getFSTIndexColumns();
-    Map<String, JsonIndexConfig> jsonIndexConfigs = 
config.getJsonIndexConfigs();
-    Map<String, H3IndexConfig> h3IndexConfigs = config.getH3IndexConfigs();
-
-    int avgNumMultiValues = config.getAvgNumMultiValues();
-
     // Metric aggregation can be enabled only if config is specified, and all 
dimensions have dictionary,
     // and no metrics have dictionary. If not enabled, the map returned is 
null.
-    _recordIdMap = enableMetricsAggregationIfPossible(config, 
noDictionaryColumns);
+    _recordIdMap = enableMetricsAggregationIfPossible(config);
 
     Map<String, Pair<String, ValueAggregator>> metricsAggregators = 
Collections.emptyMap();
     if (_recordIdMap != null) {
       metricsAggregators = getMetricsAggregators(config);
     }
 
+    Set<IndexType> specialIndexes = Sets.newHashSet(
+        StandardIndexes.dictionary(), // dictionaries implement other contract
+        StandardIndexes.nullValueVector()); // null value vector implement 
other contract

Review Comment:
   This check is actually not required because DictionaryIndexType and 
NullValueIndexType return null when their `createMutableIndex` is called, but I 
think it is cleaner in this way. We can remove that if people think otherwise



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