siddharthteotia commented on code in PR #9868:
URL: https://github.com/apache/pinot/pull/9868#discussion_r1036281464
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/ForwardIndexHandler.java:
##########
@@ -258,6 +269,31 @@ Map<String, Operation>
computeOperation(SegmentDirectory.Reader segmentReader)
return columnOperationMap;
}
+ private boolean shouldDisableDictionary(String column, ColumnMetadata
existingColumnMetadata) {
+ if (_schema == null || _indexLoadingConfig.getTableConfig() == null) {
+ // This can only happen in tests.
+ LOGGER.warn("Cannot disable dictionary for column={} as schema or
tableConfig is null.", column);
+ return false;
+ }
+
+ // Sorted columns should always have a dictionary.
+ if (existingColumnMetadata.isSorted()) {
+ LOGGER.warn("Cannot disable dictionary for column={} as it is sorted.",
column);
+ return false;
+ }
+
+ // Allow disabling dictionary only if the new config specifies that
inverted index and FST index should not
+ // be present. So for existing segments where inverted index and FST index
are already present, disabling
+ // dictionary will only be allowed if FST and inverted index are also
disabled.
+ if (_indexLoadingConfig.getInvertedIndexColumns().contains(column) ||
_indexLoadingConfig.getFSTIndexColumns()
+ .contains(column)) {
+ LOGGER.warn("Cannot disabled dictionary as column={} has FST index or
inverted index or both.", column);
Review Comment:
cc @vvivekiyer
--
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
