saurabhd336 commented on code in PR #10715:
URL: https://github.com/apache/pinot/pull/10715#discussion_r1185759408


##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/IndexReaderFactory.java:
##########
@@ -45,6 +45,18 @@ R createIndexReader(SegmentDirectory.Reader segmentReader, 
FieldIndexConfigs fie
     protected abstract R createIndexReader(PinotDataBuffer dataBuffer, 
ColumnMetadata metadata, C indexConfig)
         throws IOException, IndexReaderConstraintException;
 
+    /**
+     * Sometimes the index configuration indicates that the index should be 
disabled but the reader actually contains
+     * a buffer for the index type.
+     *
+     * By default, the buffer has priority over the configuration, so in case 
we have a buffer we would create an index

Review Comment:
   In typical pinot server loading a segment case, I honestly don't see why it 
would be a problem. When servers come up, If there's a mismatch b/w table 
config and local segment metadata, the segment is always preprocessed as per 
the latest config before loading. So post preprocessing, the segment metadata 
should always be the source of truth. i.e. you're right that at the IndexReader 
level, we can always give preference to buffer over table config.
   
   Although I'd let @Jackie-Jiang  / @npawar / @klsince share their thoughts.



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