gortiz commented on code in PR #10687:
URL: https://github.com/apache/pinot/pull/10687#discussion_r1183496362
##########
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:
> As for the dictionary, the only reason we can't use it because it's
supposed to return the dictionary id of the object being added?
Also, which value would they receive for the `dictId` argument? And how can
you ensure that the dictionary is called before the other indexes? Some hacks
can be added, but it is clear that this index is special for MutableSegmentImpl
and therefore it is easier to make that clear.
--
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
