siddharthteotia commented on code in PR #9868: URL: https://github.com/apache/pinot/pull/9868#discussion_r1035620501
########## 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: Do we still need this here if validator can catch it ? Reload is invoked after tableConfig is updated successfully so I don't think this check is needed -- 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