Merge pull request #1385 from metamx/brokenDimExtractionFilter

Fix for breaking Dimension Extraction Filter on empty filter

processing/src/main/java/io/druid/segment/filter/ExtractionFilter.java

@@ -19,7 +19,7 @@ package io.druid.segment.filter;
 
 import com.google.common.collect.Lists;
 import com.metamx.collections.bitmap.ImmutableBitmap;
-import com.metamx.collections.bitmap.WrappedConciseBitmap;
+import com.metamx.collections.bitmap.WrappedImmutableConciseBitmap;
 import io.druid.query.extraction.ExtractionFn;
 import io.druid.query.filter.BitmapIndexSelector;
 import io.druid.query.filter.Filter;
@@ -27,6 +27,7 @@ import io.druid.query.filter.ValueMatcher;
 import io.druid.query.filter.ValueMatcherFactory;
 import io.druid.segment.ColumnSelectorFactory;
 import io.druid.segment.data.Indexed;
+import it.uniroma3.mat.extendedset.intset.ImmutableConciseSet;
 
 import java.util.List;
 
@@ -65,12 +66,14 @@ public class ExtractionFilter implements Filter
     return filters;
   }
 
+  private static final WrappedImmutableConciseBitmap ZERO_LENGTH_SET = new WrappedImmutableConciseBitmap(new ImmutableConciseSet());
+
   @Override
   public ImmutableBitmap getBitmapIndex(BitmapIndexSelector selector)
   {
     final List<Filter> filters = makeFilters(selector);
     if (filters.isEmpty()) {
-      return new WrappedConciseBitmap();
+      return ZERO_LENGTH_SET;
     }
     return new OrFilter(makeFilters(selector)).getBitmapIndex(selector);
   }

processing/src/test/java/io/druid/segment/filter/ExtractionDimFilterTest.java

@@ -22,13 +22,17 @@ import com.metamx.collections.bitmap.BitmapFactory;
 import com.metamx.collections.bitmap.ConciseBitmapFactory;
 import com.metamx.collections.bitmap.ImmutableBitmap;
 import com.metamx.collections.bitmap.WrappedConciseBitmap;
+import com.metamx.collections.bitmap.WrappedImmutableConciseBitmap;
 import com.metamx.collections.spatial.ImmutableRTree;
 import io.druid.query.extraction.DimExtractionFn;
 import io.druid.query.extraction.ExtractionFn;
 import io.druid.query.filter.BitmapIndexSelector;
+import io.druid.query.filter.DimFilters;
+import io.druid.query.filter.ExtractionDimFilter;
 import io.druid.segment.data.ArrayIndexed;
 import io.druid.segment.data.Indexed;
 import it.uniroma3.mat.extendedset.intset.ConciseSet;
+import it.uniroma3.mat.extendedset.intset.ImmutableConciseSet;
 import org.junit.Assert;
 import org.junit.BeforeClass;
 import org.junit.Test;
@@ -47,18 +51,22 @@ public class ExtractionDimFilterTest
   );
 
   private static final Map<String, String> EXTRACTION_VALUES = ImmutableMap.of(
-      "foo1","extractDimVal"
+      "foo1", "extractDimVal"
   );
 
   private static ImmutableBitmap foo1BitMap;
+
   @BeforeClass
-  public static void setupStatic(){
+  public static void setupStatic()
+  {
     final ConciseSet conciseSet = new ConciseSet();
     conciseSet.add(1);
-    foo1BitMap = new WrappedConciseBitmap(conciseSet);
+    foo1BitMap = new WrappedImmutableConciseBitmap(ImmutableConciseSet.newImmutableFromMutable(conciseSet));
   }
+
   private static final BitmapIndexSelector BITMAP_INDEX_SELECTOR = new BitmapIndexSelector()
   {
+    private final ConciseBitmapFactory factory = new ConciseBitmapFactory();
     @Override
     public Indexed<String> getDimensionValues(String dimension)
     {
@@ -75,7 +83,7 @@ public class ExtractionDimFilterTest
     @Override
     public BitmapFactory getBitmapFactory()
     {
-      return new ConciseBitmapFactory();
+      return factory;
     }
 
     @Override
@@ -113,7 +121,8 @@ public class ExtractionDimFilterTest
   };
 
   @Test
-  public void testEmpty(){
+  public void testEmpty()
+  {
     ExtractionFilter extractionFilter = new ExtractionFilter(
         "foo", "NFDJUKFNDSJFNS", DIM_EXTRACTION_FN
     );
@@ -122,7 +131,8 @@ public class ExtractionDimFilterTest
   }
 
   @Test
-  public void testNull(){
+  public void testNull()
+  {
     ExtractionFilter extractionFilter = new ExtractionFilter(
         "FDHJSFFHDS", "extractDimVal", DIM_EXTRACTION_FN
     );
@@ -131,11 +141,117 @@ public class ExtractionDimFilterTest
   }
 
   @Test
-  public void testNormal(){
+  public void testNormal()
+  {
     ExtractionFilter extractionFilter = new ExtractionFilter(
         "foo", "extractDimVal", DIM_EXTRACTION_FN
     );
     ImmutableBitmap immutableBitmap = extractionFilter.getBitmapIndex(BITMAP_INDEX_SELECTOR);
     Assert.assertEquals(1, immutableBitmap.size());
   }
+
+  @Test
+  public void testOr()
+  {
+    Assert.assertEquals(
+        1, Filters.convertDimensionFilters(
+            DimFilters.or(
+                new ExtractionDimFilter(
+                    "foo",
+                    "extractDimVal",
+                    DIM_EXTRACTION_FN,
+                    null
+                )
+            )
+        ).getBitmapIndex(BITMAP_INDEX_SELECTOR).size()
+    );
+
+    Assert.assertEquals(
+        1,
+        Filters.convertDimensionFilters(
+            DimFilters.or(
+                new ExtractionDimFilter(
+                    "foo",
+                    "extractDimVal",
+                    DIM_EXTRACTION_FN,
+                    null
+                ),
+                new ExtractionDimFilter(
+                    "foo",
+                    "DOES NOT EXIST",
+                    DIM_EXTRACTION_FN,
+                    null
+                )
+            )
+        ).getBitmapIndex(BITMAP_INDEX_SELECTOR).size()
+    );
+  }
+
+  @Test
+  public void testAnd()
+  {
+    Assert.assertEquals(
+        1, Filters.convertDimensionFilters(
+            DimFilters.or(
+                new ExtractionDimFilter(
+                    "foo",
+                    "extractDimVal",
+                    DIM_EXTRACTION_FN,
+                    null
+                )
+            )
+        ).getBitmapIndex(BITMAP_INDEX_SELECTOR).size()
+    );
+
+    Assert.assertEquals(
+        1,
+        Filters.convertDimensionFilters(
+            DimFilters.and(
+                new ExtractionDimFilter(
+                    "foo",
+                    "extractDimVal",
+                    DIM_EXTRACTION_FN,
+                    null
+                ),
+                new ExtractionDimFilter(
+                    "foo",
+                    "extractDimVal",
+                    DIM_EXTRACTION_FN,
+                    null
+                )
+            )
+        ).getBitmapIndex(BITMAP_INDEX_SELECTOR).size()
+    );
+  }
+
+  @Test
+  public void testNot(){
+
+    Assert.assertEquals(
+        1, Filters.convertDimensionFilters(
+            DimFilters.or(
+                new ExtractionDimFilter(
+                    "foo",
+                    "extractDimVal",
+                    DIM_EXTRACTION_FN,
+                    null
+                )
+            )
+        ).getBitmapIndex(BITMAP_INDEX_SELECTOR).size()
+    );
+
+    Assert.assertEquals(
+        1,
+        Filters.convertDimensionFilters(
+            DimFilters.not(
+                new ExtractionDimFilter(
+                    "foo",
+                    "DOES NOT EXIST",
+                    DIM_EXTRACTION_FN,
+                    null
+                )
+            )
+        ).getBitmapIndex(BITMAP_INDEX_SELECTOR).size()
+    );
+  }
 }