gortiz commented on code in PR #10184:
URL: https://github.com/apache/pinot/pull/10184#discussion_r1153266388
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/inverted/InvertedIndexType.java:
##########
@@ -86,7 +130,46 @@ public String getFileExtension(ColumnMetadata
columnMetadata) {
}
@Override
- public String toString() {
- return getId();
+ public IndexHandler createIndexHandler(SegmentDirectory segmentDirectory,
Map<String, FieldIndexConfigs> configsByCol,
+ @Nullable Schema schema, @Nullable TableConfig tableConfig) {
+ return new InvertedIndexHandler(segmentDirectory, configsByCol,
tableConfig);
+ }
+
+ public static class ReaderFactory implements
IndexReaderFactory<InvertedIndexReader> {
+ public static final ReaderFactory INSTANCE = new ReaderFactory();
+
+ private ReaderFactory() {
+ }
+
+ @Override
+ public InvertedIndexReader createIndexReader(SegmentDirectory.Reader
segmentReader,
+ FieldIndexConfigs fieldIndexConfigs, ColumnMetadata metadata)
+ throws IOException, IndexReaderConstraintException {
+ if (fieldIndexConfigs == null ||
!fieldIndexConfigs.getConfig(StandardIndexes.inverted()).isEnabled()) {
+ return null;
+ }
+ if (!metadata.hasDictionary()) {
+ throw new IllegalStateException("Column " + metadata.getColumnName() +
" cannot be indexed by an inverted "
+ + "index if it has no dictionary");
+ }
+ if (metadata.isSorted() && metadata.isSingleValue()) {
+ ForwardIndexReader fwdReader =
StandardIndexes.forward().getReaderFactory()
+ .createIndexReader(segmentReader, fieldIndexConfigs, metadata);
+ Preconditions.checkState(fwdReader instanceof SortedIndexReader);
+ return (SortedIndexReader) fwdReader;
+ } else {
+ return createSkippingForward(segmentReader, metadata);
+ }
+ }
+
+ public InvertedIndexReader createSkippingForward(SegmentDirectory.Reader
segmentReader, ColumnMetadata metadata)
Review Comment:
That sounds dangerous. A caller may decide to call this method instead of
the other just because it has less parameters when they actually expect to get
the most efficient index and no some particular case.
I'm open to look for another name if you think it would fit better, but I
wouldn't like to just use the same name. What I will also do is to add some
javadoc.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
