[GitHub] [druid] clintropolis commented on a diff in pull request #12388: remake column indexes and query processing of filters

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

clintropolis commented on code in PR #12388:
URL: https://github.com/apache/druid/pull/12388#discussion_r847923743


##########
processing/src/main/java/org/apache/druid/query/filter/ColumnIndexSelector.java:
##########
@@ -19,33 +19,33 @@
 
 package org.apache.druid.query.filter;
 
-import com.google.errorprone.annotations.MustBeClosed;
 import org.apache.druid.collections.bitmap.BitmapFactory;
-import org.apache.druid.collections.bitmap.ImmutableBitmap;
-import org.apache.druid.collections.spatial.ImmutableRTree;
 import org.apache.druid.segment.ColumnInspector;
-import org.apache.druid.segment.column.BitmapIndex;
-import org.apache.druid.segment.column.ColumnCapabilities;
-import org.apache.druid.segment.data.CloseableIndexed;
+import org.apache.druid.segment.column.ColumnIndexCapabilities;
 
 import javax.annotation.Nullable;
 
 /**
  */
-public interface BitmapIndexSelector extends ColumnInspector
+public interface ColumnIndexSelector extends ColumnInspector
 {
-  @MustBeClosed
-  @Nullable
-  CloseableIndexed<String> getDimensionValues(String dimension);
-
-  @Deprecated
-  ColumnCapabilities.Capable hasMultipleValues(String dimension);
-
   int getNumRows();
+
   BitmapFactory getBitmapFactory();
+
+  /**
+   * Get the {@link ColumnIndexCapabilities} of a column for the specified type of index. If the index does not exist
+   * this method will return null. Note that 'missing' columns should in fact return a non-null value from this method
+   * to allow for filters to use 'nil' bitmaps if the filter matches nulls, in order to produce an all true or all
+   * false index.
+   */
   @Nullable
-  BitmapIndex getBitmapIndex(String dimension);
+  <T> ColumnIndexCapabilities getIndexCapabilities(String column, Class<T> clazz);
+
+  /**
+   * Get the specified type of index for the specified column. {@link #getIndexCapabilities(String, Class)} should
+   * be called prior to this method to distinguish 'missing' columns from columns without indexes.
+   */
   @Nullable
-  ImmutableBitmap getBitmapIndex(String dimension, String value);
-  ImmutableRTree getSpatialIndex(String dimension);
+  <T> T as(String column, Class<T> clazz);

Review Comment:
   This makes sense to just expose the index supplier here, and eliminates the need to check capabilities as a proxy for if the column exists or not. However, I think we still need index capabilities for other reasons to provide information about characteristics of the indexes themselves. It would need to return a non-null index supplier for all columns that exist, and that index supplier can just return nulls for capabilities and indexes, which is easy enough.



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

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


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