[GitHub] [pinot] klsince commented on a change in pull request #8035: refine segment consistency check

[GitBox <gi...@apache.org>]

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



##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/bloomfilter/BloomFilterHandler.java
##########
@@ -65,9 +65,25 @@ public BloomFilterHandler(SegmentMetadata segmentMetadata, IndexLoadingConfig in
 
   @Override
   public boolean needUpdateIndices(SegmentDirectory.Reader segmentReader) {
+    String segmentName = _segmentMetadata.getName();
     Set<String> columnsToAddBF = new HashSet<>(_bloomFilterConfigs.keySet());
     Set<String> existingColumns = segmentReader.toSegmentDirectory().getColumnsWithIndex(ColumnIndexType.BLOOM_FILTER);
-    return !existingColumns.equals(columnsToAddBF);
+    // Check if any existing bloomfilter need to be removed.
+    for (String column : existingColumns) {
+      if (!columnsToAddBF.remove(column)) {
+        LOGGER.debug("Need to remove existing bloom filter from segment: {}, column: {}", segmentName, column);
+        return true;
+      }
+    }
+    // Check if any new bloomfilter need to be added.
+    for (String column : columnsToAddBF) {
+      ColumnMetadata columnMetadata = _segmentMetadata.getColumnMetadataFor(column);
+      if (columnMetadata != null && columnMetadata.hasDictionary()) {

Review comment:
       Excellent point!




-- 
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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org