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


##########
processing/src/main/java/org/apache/druid/segment/filter/AndFilter.java:
##########
@@ -68,44 +73,81 @@ public AndFilter(List<Filter> filters)
   }
 
   public static <T> ImmutableBitmap getBitmapIndex(
-      BitmapIndexSelector selector,
+      ColumnIndexSelector selector,
       BitmapResultFactory<T> bitmapResultFactory,
       List<Filter> filters
   )
   {
-    return bitmapResultFactory.toImmutableBitmap(getBitmapResult(selector, 
bitmapResultFactory, filters));
+    return bitmapResultFactory.toImmutableBitmap(
+        getBitmapIndex(selector, 
filters).computeBitmapResult(bitmapResultFactory)
+    );
   }
 
-  private static <T> T getBitmapResult(
-      BitmapIndexSelector selector,
-      BitmapResultFactory<T> bitmapResultFactory,
+  private static BitmapColumnIndex getBitmapIndex(
+      ColumnIndexSelector selector,
       Collection<Filter> filters
   )
   {
     if (filters.size() == 1) {
-      return Iterables.getOnlyElement(filters).getBitmapResult(selector, 
bitmapResultFactory);
+      return Iterables.getOnlyElement(filters).getBitmapColumnIndex(selector);
     }
 
-    final List<T> bitmapResults = 
Lists.newArrayListWithCapacity(filters.size());
+    final List<BitmapColumnIndex> bitmapColumnIndices = new 
ArrayList<>(filters.size());
+    ColumnIndexCapabilities merged = new SimpleColumnIndexCapabilities(true, 
true);
     for (final Filter filter : filters) {
-      Preconditions.checkArgument(filter.supportsBitmapIndex(selector),
-                                  "Filter[%s] does not support bitmap index", 
filter
-      );
-      final T bitmapResult = filter.getBitmapResult(selector, 
bitmapResultFactory);
-      if (bitmapResultFactory.isEmpty(bitmapResult)) {
-        // Short-circuit.
-        return bitmapResultFactory.wrapAllFalse(Filters.allFalse(selector));
+      final BitmapColumnIndex index = filter.getBitmapColumnIndex(selector);
+      if (index == null) {

Review Comment:
   since this is the method on `Filter`, index here will be null for any 
sub-filter which was unable to find an index to use, like how numeric columns 
currently are since they lack indexes.
   
   At the lower level, if `ColumnIndexSupplier`, then the column doesn't exist 
(or is not filterable, which as mentioned in another comment I would sort of 
like to rework since not filterable doesn't really mean not filterable right 
now, it means all null and matches like that). All columns which exist have a 
non-null `ColumnIndexSupplier`. If the `ColumnIndexSupplier` is not null, then 
a null response from its `as` method means that the column doesn't have the 
type of index it was asked for.
   
   Back to the filter `getBitmapColumnIndex` method, this method might return a 
non-null value even if the underlying column doesn't exist 
(`ColumnIndexSupplier` was null so it can match the matcher against null to 
produce an all true or all false), or the column does exist and supports one of 
the types of indexes that the filter understands.



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

Reply via email to