diff --git a/processing/src/main/java/io/druid/segment/QueryableIndexStorageAdapter.java b/processing/src/main/java/io/druid/segment/QueryableIndexStorageAdapter.java index 24bddc89ea1f..fc1d898aa12c 100644 --- a/processing/src/main/java/io/druid/segment/QueryableIndexStorageAdapter.java +++ b/processing/src/main/java/io/druid/segment/QueryableIndexStorageAdapter.java @@ -275,13 +275,10 @@ public Sequence makeCursors(Filter filter, Interval interval, QueryGranu if (preFilters.size() == 0) { offset = new NoFilterOffset(0, index.getNumRows(), descending); } else { - List bitmaps = Lists.newArrayList(); - for (Filter prefilter : preFilters) { - bitmaps.add(prefilter.getBitmapIndex(selector)); - } + // Use AndFilter.getBitmapIndex to intersect the preFilters to get its short-circuiting behavior. offset = new BitmapOffset( selector.getBitmapFactory(), - selector.getBitmapFactory().intersection(bitmaps), + AndFilter.getBitmapIndex(selector, preFilters), descending ); } @@ -849,7 +846,8 @@ public Number get() } } final Expr.ObjectBinding binding = Parser.withSuppliers(values); - return new NumericColumnSelector() { + return new NumericColumnSelector() + { @Override public Number get() { @@ -923,6 +921,7 @@ public void reset() ); final ValueMatcher filterMatcher; + { if (postFilter instanceof BooleanFilter) { filterMatcher = ((BooleanFilter) postFilter).makeMatcher( @@ -1158,12 +1157,13 @@ public CursorOffsetHolderRowOffsetMatcherFactory(CursorOffsetHolder holder, bool // Use an iterator-based implementation, ImmutableBitmap.get(index) works differently for Concise and Roaring. // ImmutableConciseSet.get(index) is also inefficient, it performs a linear scan on each call @Override - public ValueMatcher makeRowOffsetMatcher(final ImmutableBitmap rowBitmap) { + public ValueMatcher makeRowOffsetMatcher(final ImmutableBitmap rowBitmap) + { final IntIterator iter = descending ? BitmapOffset.getReverseBitmapOffsetIterator(rowBitmap) : rowBitmap.iterator(); - if(!iter.hasNext()) { + if (!iter.hasNext()) { return new BooleanValueMatcher(false); } @@ -1252,7 +1252,8 @@ public void increment() } @Override - public Offset clone() { + public Offset clone() + { throw new IllegalStateException("clone"); } } diff --git a/processing/src/main/java/io/druid/segment/filter/AndFilter.java b/processing/src/main/java/io/druid/segment/filter/AndFilter.java index 56fc83af8445..778a83e3cfcd 100644 --- a/processing/src/main/java/io/druid/segment/filter/AndFilter.java +++ b/processing/src/main/java/io/druid/segment/filter/AndFilter.java @@ -47,21 +47,31 @@ public AndFilter( this.filters = filters; } - @Override - public ImmutableBitmap getBitmapIndex(BitmapIndexSelector selector) + public static ImmutableBitmap getBitmapIndex(BitmapIndexSelector selector, List filters) { if (filters.size() == 1) { return filters.get(0).getBitmapIndex(selector); } - List bitmaps = Lists.newArrayList(); - for (int i = 0; i < filters.size(); i++) { - bitmaps.add(filters.get(i).getBitmapIndex(selector)); + final List bitmaps = Lists.newArrayListWithCapacity(filters.size()); + for (final Filter filter : filters) { + final ImmutableBitmap bitmapIndex = filter.getBitmapIndex(selector); + if (bitmapIndex.isEmpty()) { + // Short-circuit. + return Filters.allFalse(selector); + } + bitmaps.add(bitmapIndex); } return selector.getBitmapFactory().intersection(bitmaps); } + @Override + public ImmutableBitmap getBitmapIndex(BitmapIndexSelector selector) + { + return getBitmapIndex(selector, filters); + } + @Override public ValueMatcher makeMatcher(ValueMatcherFactory factory) { diff --git a/processing/src/test/java/io/druid/segment/filter/AndFilterTest.java b/processing/src/test/java/io/druid/segment/filter/AndFilterTest.java new file mode 100644 index 000000000000..1b9d16c27969 --- /dev/null +++ b/processing/src/test/java/io/druid/segment/filter/AndFilterTest.java @@ -0,0 +1,188 @@ +/* + * Licensed to Metamarkets Group Inc. (Metamarkets) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Metamarkets licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package io.druid.segment.filter; + +import com.google.common.base.Function; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import io.druid.data.input.InputRow; +import io.druid.data.input.impl.DimensionsSpec; +import io.druid.data.input.impl.InputRowParser; +import io.druid.data.input.impl.MapInputRowParser; +import io.druid.data.input.impl.TimeAndDimsParseSpec; +import io.druid.data.input.impl.TimestampSpec; +import io.druid.java.util.common.Pair; +import io.druid.query.filter.AndDimFilter; +import io.druid.query.filter.DimFilter; +import io.druid.query.filter.NotDimFilter; +import io.druid.query.filter.SelectorDimFilter; +import io.druid.segment.IndexBuilder; +import io.druid.segment.StorageAdapter; +import org.joda.time.DateTime; +import org.junit.AfterClass; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +import java.io.Closeable; +import java.util.List; +import java.util.Map; + +@RunWith(Parameterized.class) +public class AndFilterTest extends BaseFilterTest +{ + private static final String TIMESTAMP_COLUMN = "timestamp"; + + private static final InputRowParser> PARSER = new MapInputRowParser( + new TimeAndDimsParseSpec( + new TimestampSpec(TIMESTAMP_COLUMN, "iso", new DateTime("2000")), + new DimensionsSpec(null, null, null) + ) + ); + + private static final List ROWS = ImmutableList.of( + PARSER.parse(ImmutableMap.of("dim0", "0", "dim1", "0")), + PARSER.parse(ImmutableMap.of("dim0", "1", "dim1", "0")), + PARSER.parse(ImmutableMap.of("dim0", "2", "dim1", "0")), + PARSER.parse(ImmutableMap.of("dim0", "3", "dim1", "0")), + PARSER.parse(ImmutableMap.of("dim0", "4", "dim1", "0")), + PARSER.parse(ImmutableMap.of("dim0", "5", "dim1", "0")) + ); + + public AndFilterTest( + String testName, + IndexBuilder indexBuilder, + Function> finisher, + boolean optimize + ) + { + super(testName, ROWS, indexBuilder, finisher, optimize); + } + + @AfterClass + public static void tearDown() throws Exception + { + BaseFilterTest.tearDown(AndFilterTest.class.getName()); + } + + @Test + public void testAnd() + { + assertFilterMatches( + new AndDimFilter(ImmutableList.of( + new SelectorDimFilter("dim0", "0", null), + new SelectorDimFilter("dim1", "0", null) + )), + ImmutableList.of("0") + ); + assertFilterMatches( + new AndDimFilter(ImmutableList.of( + new SelectorDimFilter("dim0", "0", null), + new SelectorDimFilter("dim1", "1", null) + )), + ImmutableList.of() + ); + assertFilterMatches( + new AndDimFilter(ImmutableList.of( + new SelectorDimFilter("dim0", "1", null), + new SelectorDimFilter("dim1", "0", null) + )), + ImmutableList.of("1") + ); + assertFilterMatches( + new AndDimFilter(ImmutableList.of( + new SelectorDimFilter("dim0", "1", null), + new SelectorDimFilter("dim1", "1", null) + )), + ImmutableList.of() + ); + assertFilterMatches( + new AndDimFilter(ImmutableList.of( + new NotDimFilter(new SelectorDimFilter("dim0", "1", null)), + new NotDimFilter(new SelectorDimFilter("dim1", "1", null)) + )), + ImmutableList.of("0", "2", "3", "4", "5") + ); + assertFilterMatches( + new AndDimFilter(ImmutableList.of( + new NotDimFilter(new SelectorDimFilter("dim0", "0", null)), + new NotDimFilter(new SelectorDimFilter("dim1", "0", null)) + )), + ImmutableList.of() + ); + } + + @Test + public void testNotAnd() + { + assertFilterMatches( + new NotDimFilter(new AndDimFilter(ImmutableList.of( + new SelectorDimFilter("dim0", "0", null), + new SelectorDimFilter("dim1", "0", null) + ))), + ImmutableList.of("1", "2", "3", "4", "5") + ); + assertFilterMatches( + new NotDimFilter(new AndDimFilter(ImmutableList.of( + new SelectorDimFilter("dim0", "0", null), + new SelectorDimFilter("dim1", "1", null) + ))), + ImmutableList.of("0", "1", "2", "3", "4", "5") + ); + assertFilterMatches( + new NotDimFilter(new AndDimFilter(ImmutableList.of( + new SelectorDimFilter("dim0", "1", null), + new SelectorDimFilter("dim1", "0", null) + ))), + ImmutableList.of("0", "2", "3", "4", "5") + ); + assertFilterMatches( + new NotDimFilter(new AndDimFilter(ImmutableList.of( + new SelectorDimFilter("dim0", "1", null), + new SelectorDimFilter("dim1", "1", null) + ))), + ImmutableList.of("0", "1", "2", "3", "4", "5") + ); + assertFilterMatches( + new NotDimFilter(new AndDimFilter(ImmutableList.of( + new NotDimFilter(new SelectorDimFilter("dim0", "1", null)), + new NotDimFilter(new SelectorDimFilter("dim1", "1", null)) + ))), + ImmutableList.of("1") + ); + assertFilterMatches( + new NotDimFilter(new AndDimFilter(ImmutableList.of( + new NotDimFilter(new SelectorDimFilter("dim0", "0", null)), + new NotDimFilter(new SelectorDimFilter("dim1", "0", null)) + ))), + ImmutableList.of("0", "1", "2", "3", "4", "5") + ); + } + + private void assertFilterMatches( + final DimFilter filter, + final List expectedRows + ) + { + Assert.assertEquals(filter.toString(), expectedRows, selectColumnValuesMatchingFilter(filter, "dim0")); + Assert.assertEquals(filter.toString(), expectedRows.size(), selectCountUsingFilteredAggregator(filter)); + } +}