clintropolis commented on a change in pull request #12315:
URL: https://github.com/apache/druid/pull/12315#discussion_r822452684



##########
File path: 
processing/src/main/java/org/apache/druid/segment/virtual/ListFilteredVirtualColumn.java
##########
@@ -252,22 +256,172 @@ public BitmapFactory getBitmapFactory()
       return delegate.getBitmapFactory();
     }
 
+    @Override
+    public int getCardinality()
+    {
+      return idMapping.getValueCardinality();
+    }
+
+    @Override
+    public int getIndex(@Nullable String value)
+    {
+      return getReverseIndex(value);
+    }
+
     @Override
     public ImmutableBitmap getBitmap(int idx)
     {
       return delegate.getBitmap(idMapping.getReverseId(idx));
     }
 
     @Override
-    public int getCardinality()
+    public ImmutableBitmap getBitmapForValue(@Nullable String value)
     {
-      return idMapping.getValueCardinality();
+      if (getReverseIndex(value) < 0) {
+        return delegate.getBitmap(-1);
+      }
+      return delegate.getBitmapForValue(value);
     }
 
     @Override
-    public int getIndex(@Nullable String value)
+    public Iterable<ImmutableBitmap> getBitmapsInRange(

Review comment:
       >For the "include" case, the implementations of these methods seem like 
they should be able to be a lot simpler. The normal use case provides a much 
smaller list of things to show, it seems like we could quite rapidly filter 
through that list and then delegate to something that uses that list.
   
   I _think_ this is already doing this, the range iterated over here is the 
cardinality of the idLookup, not the dictionary of the delegate column, so it 
only considers ranges of the filtered values. There might be room for 
improvement, especially in the deny case, though I think it probably would need 
bigger changes to `ListFilteredVirtualColumn` and maybe best to do as a 
follow-up.




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