gortiz commented on code in PR #10715:
URL: https://github.com/apache/pinot/pull/10715#discussion_r1185818858
##########
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:
What about this approach? Now the decision between prioritizing buffers or
not is delegated to the caller. By default indexes will honor the config, but
dictionary and null value will not.
I've also found that PhysicalColumnIndexContainer does not load range index
when the column is sorted, while RangeIndexType does. I'm going to leave it as
it is right now. I don't think we should add that condition into RangeIndexType
and I would prefer to do not add a special case in
PhysicalColumnIndexContainer. I assume the reason why
PhysicalColumnIndexContainer did that was because it wasn't actually necessary
to use the range index in that situation, but I guess that should be a
responsibility of the planer.
--
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
