From e1399b7ce4b7477dba8a4217c8fb26fff5263c14 Mon Sep 17 00:00:00 2001 From: Charles Allen Date: Fri, 22 May 2015 15:02:11 -0700 Subject: [PATCH 1/2] Add unit test to show breaking Dimension Extraction Filter --- .../filter/ExtractionDimFilterTest.java | 55 +++++++++++++++++-- 1 file changed, 50 insertions(+), 5 deletions(-) diff --git a/processing/src/test/java/io/druid/segment/filter/ExtractionDimFilterTest.java b/processing/src/test/java/io/druid/segment/filter/ExtractionDimFilterTest.java index f58436e666d..0ece0b65dbb 100644 --- a/processing/src/test/java/io/druid/segment/filter/ExtractionDimFilterTest.java +++ b/processing/src/test/java/io/druid/segment/filter/ExtractionDimFilterTest.java @@ -26,6 +26,8 @@ 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; @@ -47,16 +49,19 @@ public class ExtractionDimFilterTest ); private static final Map 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); } + private static final BitmapIndexSelector BITMAP_INDEX_SELECTOR = new BitmapIndexSelector() { @Override @@ -113,7 +118,8 @@ public class ExtractionDimFilterTest }; @Test - public void testEmpty(){ + public void testEmpty() + { ExtractionFilter extractionFilter = new ExtractionFilter( "foo", "NFDJUKFNDSJFNS", DIM_EXTRACTION_FN ); @@ -122,7 +128,8 @@ public class ExtractionDimFilterTest } @Test - public void testNull(){ + public void testNull() + { ExtractionFilter extractionFilter = new ExtractionFilter( "FDHJSFFHDS", "extractDimVal", DIM_EXTRACTION_FN ); @@ -131,11 +138,49 @@ 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() + ); + } } From e97d22a10a33c793b52314f50160f429e2d0c596 Mon Sep 17 00:00:00 2001 From: Charles Allen Date: Fri, 22 May 2015 15:20:11 -0700 Subject: [PATCH 2/2] Fix Extraction Filter cast problems for empty results --- .../segment/filter/ExtractionFilter.java | 7 +- .../filter/ExtractionDimFilterTest.java | 75 ++++++++++++++++++- 2 files changed, 78 insertions(+), 4 deletions(-) diff --git a/processing/src/main/java/io/druid/segment/filter/ExtractionFilter.java b/processing/src/main/java/io/druid/segment/filter/ExtractionFilter.java index 906b1adb3fd..749b441d005 100644 --- a/processing/src/main/java/io/druid/segment/filter/ExtractionFilter.java +++ b/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 filters = makeFilters(selector); if (filters.isEmpty()) { - return new WrappedConciseBitmap(); + return ZERO_LENGTH_SET; } return new OrFilter(makeFilters(selector)).getBitmapIndex(selector); } diff --git a/processing/src/test/java/io/druid/segment/filter/ExtractionDimFilterTest.java b/processing/src/test/java/io/druid/segment/filter/ExtractionDimFilterTest.java index 0ece0b65dbb..7d3370a55c5 100644 --- a/processing/src/test/java/io/druid/segment/filter/ExtractionDimFilterTest.java +++ b/processing/src/test/java/io/druid/segment/filter/ExtractionDimFilterTest.java @@ -22,6 +22,7 @@ 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; @@ -31,6 +32,7 @@ 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; @@ -59,11 +61,12 @@ public class ExtractionDimFilterTest { 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 getDimensionValues(String dimension) { @@ -80,7 +83,7 @@ public class ExtractionDimFilterTest @Override public BitmapFactory getBitmapFactory() { - return new ConciseBitmapFactory(); + return factory; } @Override @@ -183,4 +186,72 @@ public class ExtractionDimFilterTest ).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() + ); + } }