From 214125e15bdf08a69b830459f4a5bc67da03ac38 Mon Sep 17 00:00:00 2001 From: "navis.ryu" Date: Tue, 16 Feb 2016 10:36:01 +0900 Subject: [PATCH 1/2] Support simple binary operator for dimension filter --- docs/content/querying/filters.md | 6 +- .../src/main/java/io/druid/query/Druids.java | 11 +- .../io/druid/query/filter/BinaryOperator.java | 133 ++++++++++++++++++ .../druid/query/filter/SelectorDimFilter.java | 32 ++++- .../io/druid/segment/data/ArrayIndexed.java | 2 +- .../java/io/druid/segment/filter/Filters.java | 7 +- .../druid/segment/filter/SelectorFilter.java | 116 ++++++++++++--- .../io/druid/query/QueryRunnerTestHelper.java | 34 ++++- .../search/SearchQueryRunnerWithCaseTest.java | 8 +- .../query/select/SelectQueryRunnerTest.java | 110 ++++++++++++++- .../timeseries/TimeseriesQueryRunnerTest.java | 57 ++++++++ 11 files changed, 478 insertions(+), 38 deletions(-) create mode 100644 processing/src/main/java/io/druid/query/filter/BinaryOperator.java diff --git a/docs/content/querying/filters.md b/docs/content/querying/filters.md index 629151069f49..9ce827c0ea23 100644 --- a/docs/content/querying/filters.md +++ b/docs/content/querying/filters.md @@ -11,10 +11,12 @@ The simplest filter is a selector filter. The selector filter will match a speci The grammar for a SELECTOR filter is as follows: ``` json -"filter": { "type": "selector", "dimension": , "value": } +"filter": { "type": "selector", "dimension": , "value": , "operator": } ``` -This is the equivalent of `WHERE = ''`. +When `operator` is not specified it will be translated to `EQUAL` operator. +`operator` can be one of `GT`, `GreaterThan`, `>=`, `GTE`, `GreaterThanOrEqualTo`, `>`, `LT`, `LessThan`, `<`, `LTE`, `LessThanOrEqualTo`, `<=`, `EQ`, `Equals`, `==`, `NE`, `NotEquals`, `!=`, `<>`. Except `EQUAL` and `NOT_EQUAL` operator, null or empty value cannot be used. +This is the equivalent of `WHERE ''`. ### Regular expression filter diff --git a/processing/src/main/java/io/druid/query/Druids.java b/processing/src/main/java/io/druid/query/Druids.java index fe6c096160e5..0c91beae24ea 100644 --- a/processing/src/main/java/io/druid/query/Druids.java +++ b/processing/src/main/java/io/druid/query/Druids.java @@ -244,23 +244,26 @@ public static NotDimFilterBuilder newNotDimFilterBuilder() public static class SelectorDimFilterBuilder { private String dimension; + private String operation; private String value; public SelectorDimFilterBuilder() { dimension = ""; + operation = ""; value = ""; } public SelectorDimFilter build() { - return new SelectorDimFilter(dimension, value); + return new SelectorDimFilter(dimension, value, operation); } public SelectorDimFilterBuilder copy(SelectorDimFilterBuilder builder) { return new SelectorDimFilterBuilder() .dimension(builder.dimension) + .operation(builder.operation) .value(builder.value); } @@ -270,6 +273,12 @@ public SelectorDimFilterBuilder dimension(String d) return this; } + public SelectorDimFilterBuilder operation(String op) + { + operation = op; + return this; + } + public SelectorDimFilterBuilder value(String v) { value = v; diff --git a/processing/src/main/java/io/druid/query/filter/BinaryOperator.java b/processing/src/main/java/io/druid/query/filter/BinaryOperator.java new file mode 100644 index 000000000000..e95c31966e73 --- /dev/null +++ b/processing/src/main/java/io/druid/query/filter/BinaryOperator.java @@ -0,0 +1,133 @@ +package io.druid.query.filter; + +import com.google.common.base.Predicate; +import com.google.common.base.Strings; + +/** + */ +public enum BinaryOperator +{ + GT { + @Override + public Predicate toPredicate(final String value) + { + return new Predicate() + { + @Override + public boolean apply(String input) + { + return input.compareTo(value) > 0; + } + }; + } + }, + GTE { + @Override + public Predicate toPredicate(final String value) + { + return new Predicate() + { + @Override + public boolean apply(String input) + { + return input.compareTo(value) >= 0; + } + }; + } + }, + LT { + @Override + public Predicate toPredicate(final String value) + { + return new Predicate() + { + @Override + public boolean apply(String input) + { + return input.compareTo(value) < 0; + } + }; + } + }, + LTE { + @Override + public Predicate toPredicate(final String value) + { + return new Predicate() + { + @Override + public boolean apply(String input) + { + return input.compareTo(value) <= 0; + } + }; + } + }, + EQ { + @Override + public Predicate toPredicate(final String value) + { + return new Predicate() + { + @Override + public boolean apply(String input) + { + return input.equals(value); + } + }; + } + }, + NE { + @Override + public Predicate toPredicate(final String value) + { + return new Predicate() + { + @Override + public boolean apply(String input) + { + return !input.equals(value); + } + }; + } + }; + + public abstract Predicate toPredicate(final String value); + + public static BinaryOperator get(String value) + { + if (Strings.isNullOrEmpty(value)) { + return EQ; + } + value = value.toLowerCase(); + switch (value) { + case "gt": + case "greaterthan": + case ">": + return GT; + case "gte": + case "greaterthanorequalto": + case ">=": + return GTE; + case "lt": + case "lessthan": + case "<": + return LT; + case "lte": + case "lessthanorequalto": + case "<=": + return LTE; + case "eq": + case "equals": + case "=": + case "==": + return EQ; + case "ne": + case "notequals": + case "!=": + case "<>": + return NE; + } + throw new IllegalArgumentException("Invalid operator " + value); + } +} diff --git a/processing/src/main/java/io/druid/query/filter/SelectorDimFilter.java b/processing/src/main/java/io/druid/query/filter/SelectorDimFilter.java index 93b0002016e1..758e85420d16 100644 --- a/processing/src/main/java/io/druid/query/filter/SelectorDimFilter.java +++ b/processing/src/main/java/io/druid/query/filter/SelectorDimFilter.java @@ -22,6 +22,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Preconditions; +import com.google.common.base.Strings; import com.metamx.common.StringUtils; import java.nio.ByteBuffer; @@ -32,17 +33,31 @@ public class SelectorDimFilter implements DimFilter { private final String dimension; private final String value; + private final BinaryOperator operator; @JsonCreator public SelectorDimFilter( @JsonProperty("dimension") String dimension, - @JsonProperty("value") String value + @JsonProperty("value") String value, + @JsonProperty("operator") String operator ) { Preconditions.checkArgument(dimension != null, "dimension must not be null"); this.dimension = dimension; this.value = value; + this.operator = BinaryOperator.get(operator); + + // don't allow null comparison, for now + Preconditions.checkArgument( + !(this.operator != BinaryOperator.EQ && this.operator != BinaryOperator.NE && Strings.isNullOrEmpty(value)), + "null comparison is not allowed, except equals/not-equals" + ); + } + + public SelectorDimFilter(String dimension, String value) + { + this(dimension, value, null); } @Override @@ -51,8 +66,9 @@ public byte[] getCacheKey() byte[] dimensionBytes = StringUtils.toUtf8(dimension); byte[] valueBytes = (value == null) ? new byte[]{} : StringUtils.toUtf8(value); - return ByteBuffer.allocate(2 + dimensionBytes.length + valueBytes.length) + return ByteBuffer.allocate(3 + dimensionBytes.length + valueBytes.length) .put(DimFilterCacheHelper.SELECTOR_CACHE_ID) + .put((byte) operator.ordinal()) .put(dimensionBytes) .put(DimFilterCacheHelper.STRING_SEPARATOR) .put(valueBytes) @@ -71,6 +87,12 @@ public String getDimension() return dimension; } + @JsonProperty + public String getOperator() + { + return operator.name(); + } + @JsonProperty public String getValue() { @@ -95,6 +117,9 @@ public boolean equals(Object o) if (value != null ? !value.equals(that.value) : that.value != null) { return false; } + if (!operator.equals(that.operator)) { + return false; + } return true; } @@ -104,12 +129,13 @@ public int hashCode() { int result = dimension != null ? dimension.hashCode() : 0; result = 31 * result + (value != null ? value.hashCode() : 0); + result = 31 * result + operator.ordinal(); return result; } @Override public String toString() { - return String.format("%s = %s", dimension, value); + return String.format("%s %s %s", dimension, operator.name(), value); } } diff --git a/processing/src/main/java/io/druid/segment/data/ArrayIndexed.java b/processing/src/main/java/io/druid/segment/data/ArrayIndexed.java index 81a594e2f562..c53afa546f4b 100644 --- a/processing/src/main/java/io/druid/segment/data/ArrayIndexed.java +++ b/processing/src/main/java/io/druid/segment/data/ArrayIndexed.java @@ -59,7 +59,7 @@ public T get(int index) @Override public int indexOf(T value) { - return Arrays.asList(baseArray).indexOf(value); + return Arrays.binarySearch(baseArray, value); } @Override diff --git a/processing/src/main/java/io/druid/segment/filter/Filters.java b/processing/src/main/java/io/druid/segment/filter/Filters.java index b9b4910deaf9..96c4fde75589 100644 --- a/processing/src/main/java/io/druid/segment/filter/Filters.java +++ b/processing/src/main/java/io/druid/segment/filter/Filters.java @@ -72,8 +72,11 @@ public static Filter convertDimensionFilters(DimFilter dimFilter) filter = new NotFilter(convertDimensionFilters(((NotDimFilter) dimFilter).getField())); } else if (dimFilter instanceof SelectorDimFilter) { final SelectorDimFilter selectorDimFilter = (SelectorDimFilter) dimFilter; - - filter = new SelectorFilter(selectorDimFilter.getDimension(), selectorDimFilter.getValue()); + filter = new SelectorFilter( + selectorDimFilter.getDimension(), + selectorDimFilter.getValue(), + selectorDimFilter.getOperator() + ); } else if (dimFilter instanceof ExtractionDimFilter) { final ExtractionDimFilter extractionDimFilter = (ExtractionDimFilter) dimFilter; diff --git a/processing/src/main/java/io/druid/segment/filter/SelectorFilter.java b/processing/src/main/java/io/druid/segment/filter/SelectorFilter.java index 861eb35e245f..4a59596492e8 100644 --- a/processing/src/main/java/io/druid/segment/filter/SelectorFilter.java +++ b/processing/src/main/java/io/druid/segment/filter/SelectorFilter.java @@ -19,15 +19,19 @@ package io.druid.segment.filter; +import com.google.common.base.Predicate; import com.google.common.base.Strings; +import com.metamx.collections.bitmap.BitmapFactory; import com.metamx.collections.bitmap.ImmutableBitmap; import io.druid.query.dimension.DefaultDimensionSpec; +import io.druid.query.filter.BinaryOperator; import io.druid.query.filter.BitmapIndexSelector; import io.druid.query.filter.Filter; import io.druid.query.filter.ValueMatcher; import io.druid.query.filter.ValueMatcherFactory; import io.druid.segment.ColumnSelectorFactory; import io.druid.segment.DimensionSelector; +import io.druid.segment.data.Indexed; import io.druid.segment.data.IndexedInts; /** @@ -36,26 +40,66 @@ public class SelectorFilter implements Filter { private final String dimension; private final String value; + private final BinaryOperator operator; public SelectorFilter( String dimension, - String value + String value, + String operator ) { this.dimension = dimension; this.value = value; + this.operator = BinaryOperator.get(operator); + } + + public SelectorFilter(String dimension, String value) + { + this(dimension, value, null); } @Override public ImmutableBitmap getBitmapIndex(BitmapIndexSelector selector) { - return selector.getBitmapIndex(dimension, value); + final BitmapFactory factory = selector.getBitmapFactory(); + final Indexed values = selector.getDimensionValues(dimension); + switch (operator) { + case GT: + case GTE: { + int index = values.indexOf(value); + int start = index < 0 ? -index - 1 : operator == BinaryOperator.GT ? index + 1: index; + ImmutableBitmap bitmap = factory.makeEmptyImmutableBitmap(); + for (int cursor = start; cursor < values.size(); cursor++) { + bitmap = bitmap.union(selector.getBitmapIndex(dimension, values.get(cursor))); + } + return bitmap; + } + case LT: + case LTE: { + int index = values.indexOf(value); + int limit = index < 0 ? -index - 2 : operator == BinaryOperator.LT ? index - 1: index; + ImmutableBitmap bitmap = factory.makeEmptyImmutableBitmap(); + for (int cursor = 0; cursor <= limit; cursor++) { + bitmap = bitmap.union(selector.getBitmapIndex(dimension, values.get(cursor))); + } + return bitmap; + } + case EQ: + return selector.getBitmapIndex(dimension, value); + case NE: + return factory.complement(selector.getBitmapIndex(dimension, value), selector.getNumRows()); + } + throw new IllegalArgumentException("Not supported operator " + operator); } @Override public ValueMatcher makeMatcher(ValueMatcherFactory factory) { - return factory.makeValueMatcher(dimension, value); + if (operator == BinaryOperator.EQ || operator == BinaryOperator.NE) { + final ValueMatcher matcher = factory.makeValueMatcher(dimension, value); + return operator == BinaryOperator.EQ ? matcher : new RevertedMatcher(matcher); + } + return factory.makeValueMatcher(dimension, operator.toPredicate(value)); } @Override @@ -66,27 +110,63 @@ public ValueMatcher makeMatcher(ColumnSelectorFactory columnSelectorFactory) ); // Missing columns match a null or empty string value and don't match anything else + if (operator == BinaryOperator.EQ || operator == BinaryOperator.NE) { + final ValueMatcher matcher = toValueMatcher(dimensionSelector); + return operator == BinaryOperator.EQ ? matcher : new RevertedMatcher(matcher); + } + + final Predicate predicate = operator.toPredicate(value); + return new ValueMatcher() + { + @Override + public boolean matches() + { + final IndexedInts row = dimensionSelector.getRow(); + final int size = row.size(); + for (int i = 0; i < size; ++i) { + if (predicate.apply(dimensionSelector.lookupName(row.get(i)))) { + return true; + } + } + return false; + } + }; + } + + private ValueMatcher toValueMatcher(final DimensionSelector dimensionSelector) + { if (dimensionSelector == null) { return new BooleanValueMatcher(Strings.isNullOrEmpty(value)); - } else { - final int valueId = dimensionSelector.lookupId(value); - return new ValueMatcher() + } + final int valueId = dimensionSelector.lookupId(value); + return new ValueMatcher() + { + @Override + public boolean matches() { - @Override - public boolean matches() - { - final IndexedInts row = dimensionSelector.getRow(); - final int size = row.size(); - for (int i = 0; i < size; ++i) { - if (row.get(i) == valueId) { - return true; - } + final IndexedInts row = dimensionSelector.getRow(); + final int size = row.size(); + for (int i = 0; i < size; ++i) { + if (row.get(i) == valueId) { + return true; } - return false; } - }; - } + return false; + } + }; } + // for NE + private static class RevertedMatcher implements ValueMatcher + { + private final ValueMatcher matcher; + + private RevertedMatcher(ValueMatcher matcher) {this.matcher = matcher;} + @Override + public boolean matches() + { + return !matcher.matches(); + } + } } diff --git a/processing/src/test/java/io/druid/query/QueryRunnerTestHelper.java b/processing/src/test/java/io/druid/query/QueryRunnerTestHelper.java index e7987d5f6d35..19223037532e 100644 --- a/processing/src/test/java/io/druid/query/QueryRunnerTestHelper.java +++ b/processing/src/test/java/io/druid/query/QueryRunnerTestHelper.java @@ -314,9 +314,9 @@ public static > List> makeQueryRunn final QueryableIndex mMappedTestIndex = TestIndex.getMMappedTestIndex(); final QueryableIndex mergedRealtimeIndex = TestIndex.mergedRealtimeIndex(); return ImmutableList.of( - makeQueryRunner(factory, new IncrementalIndexSegment(rtIndex, segmentId)), - makeQueryRunner(factory, new QueryableIndexSegment(segmentId, mMappedTestIndex)), - makeQueryRunner(factory, new QueryableIndexSegment(segmentId, mergedRealtimeIndex)) + makeQueryRunner(factory, new IncrementalIndexSegment(rtIndex, segmentId), "incremental"), + makeQueryRunner(factory, new QueryableIndexSegment(segmentId, mMappedTestIndex), "mmapped"), + makeQueryRunner(factory, new QueryableIndexSegment(segmentId, mergedRealtimeIndex), "merged") ); } @@ -398,15 +398,32 @@ public void remove() public static > QueryRunner makeQueryRunner( QueryRunnerFactory factory, Segment adapter + ) { + return makeQueryRunner(factory, adapter, null); + } + + public static > QueryRunner makeQueryRunner( + QueryRunnerFactory factory, + Segment adapter, + String name ) { - return makeQueryRunner(factory, segmentId, adapter); + return makeQueryRunner(factory, segmentId, adapter, name); } public static > QueryRunner makeQueryRunner( QueryRunnerFactory factory, String segmentId, - Segment adapter + final Segment adapter + ) { + return makeQueryRunner(factory, segmentId, adapter, null); + } + + public static > QueryRunner makeQueryRunner( + QueryRunnerFactory factory, + String segmentId, + final Segment adapter, + final String name ) { return new FinalizeResultsQueryRunner( @@ -415,7 +432,12 @@ public static > QueryRunner makeQueryRunner( factory.createRunner(adapter) ), (QueryToolChest>)factory.getToolchest() - ); + ) { + @Override + public String toString() { + return name != null ? name : super.toString(); + } + }; } public static QueryRunner makeUnionQueryRunner( diff --git a/processing/src/test/java/io/druid/query/search/SearchQueryRunnerWithCaseTest.java b/processing/src/test/java/io/druid/query/search/SearchQueryRunnerWithCaseTest.java index c9332adf536f..3fccb3d515ac 100644 --- a/processing/src/test/java/io/druid/query/search/SearchQueryRunnerWithCaseTest.java +++ b/processing/src/test/java/io/druid/query/search/SearchQueryRunnerWithCaseTest.java @@ -91,10 +91,10 @@ public static Iterable constructorFeeder() throws IOException return transformToConstructionFeeder( Arrays.asList( - makeQueryRunner(factory, "index1", new IncrementalIndexSegment(index1, "index1")), - makeQueryRunner(factory, "index2", new IncrementalIndexSegment(index2, "index2")), - makeQueryRunner(factory, "index3", new QueryableIndexSegment("index3", index3)), - makeQueryRunner(factory, "index4", new QueryableIndexSegment("index4", index4)) + makeQueryRunner(factory, "index1", new IncrementalIndexSegment(index1, "index1"), "incremental1"), + makeQueryRunner(factory, "index2", new IncrementalIndexSegment(index2, "index2"), "incremental2"), + makeQueryRunner(factory, "index3", new QueryableIndexSegment("index3", index3), "mmaped1"), + makeQueryRunner(factory, "index4", new QueryableIndexSegment("index4", index4), "mmaped2") ) ); } diff --git a/processing/src/test/java/io/druid/query/select/SelectQueryRunnerTest.java b/processing/src/test/java/io/druid/query/select/SelectQueryRunnerTest.java index 3fc958063c86..314bb95c9265 100644 --- a/processing/src/test/java/io/druid/query/select/SelectQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/select/SelectQueryRunnerTest.java @@ -28,6 +28,7 @@ import com.metamx.common.ISE; import com.metamx.common.guava.Sequences; import io.druid.jackson.DefaultObjectMapper; +import io.druid.query.Druids; import io.druid.query.QueryRunner; import io.druid.query.QueryRunnerTestHelper; import io.druid.query.Result; @@ -39,6 +40,7 @@ import io.druid.query.extraction.MapLookupExtractor; import io.druid.query.filter.AndDimFilter; import io.druid.query.filter.DimFilter; +import io.druid.query.filter.JavaScriptDimFilter; import io.druid.query.filter.SelectorDimFilter; import io.druid.query.spec.LegacySegmentSpec; import io.druid.query.spec.QuerySegmentSpec; @@ -283,6 +285,112 @@ public void testFullOnSelectWithDimensionSpec() verify(descending ? expectedResultsDsc : expectedResultsAsc, results); } + @Test + public void testSelectWithSelectDimFilter() + { + Druids.SelectQueryBuilder builder = new Druids.SelectQueryBuilder() + .dataSource(new TableDataSource(QueryRunnerTestHelper.dataSource)) + .intervals(I_0112_0114) + .descending(descending) + .granularity(QueryRunnerTestHelper.allGran) + .dimensionSpecs(DefaultDimensionSpec.toSpec(Arrays.asList(QueryRunnerTestHelper.qualityDimension))) + .metrics(Arrays.asList(QueryRunnerTestHelper.indexMetric)) + .pagingSpec(new PagingSpec(null, 20)); + + // existing + validateSelectDimFilter( + builder, + new SelectorDimFilter(QueryRunnerTestHelper.qualityDimension, "entertainment", "GT"), + new JavaScriptDimFilter(QueryRunnerTestHelper.qualityDimension, "function(dim){ return dim > 'entertainment'; }") + ); + validateSelectDimFilter( + builder, + new SelectorDimFilter(QueryRunnerTestHelper.qualityDimension, "entertainment", "GTE"), + new JavaScriptDimFilter(QueryRunnerTestHelper.qualityDimension, "function(dim){ return dim >= 'entertainment'; }") + ); + validateSelectDimFilter( + builder, + new SelectorDimFilter(QueryRunnerTestHelper.qualityDimension, "entertainment", "LT"), + new JavaScriptDimFilter(QueryRunnerTestHelper.qualityDimension, "function(dim){ return dim < 'entertainment'; }") + ); + validateSelectDimFilter( + builder, + new SelectorDimFilter(QueryRunnerTestHelper.qualityDimension, "entertainment", "LTE"), + new JavaScriptDimFilter(QueryRunnerTestHelper.qualityDimension, "function(dim){ return dim <= 'entertainment'; }") + ); + validateSelectDimFilter( + builder, + new SelectorDimFilter(QueryRunnerTestHelper.qualityDimension, "entertainment", "EQ"), + new JavaScriptDimFilter(QueryRunnerTestHelper.qualityDimension, "function(dim){ return dim === 'entertainment'; }") + ); + validateSelectDimFilter( + builder, + new SelectorDimFilter(QueryRunnerTestHelper.qualityDimension, "entertainment", "NE"), + new JavaScriptDimFilter(QueryRunnerTestHelper.qualityDimension, "function(dim){ return dim != 'entertainment'; }") + ); + + // non-existing + validateSelectDimFilter( + builder, + new SelectorDimFilter(QueryRunnerTestHelper.qualityDimension, "financial", "GT"), + new JavaScriptDimFilter(QueryRunnerTestHelper.qualityDimension, "function(dim){ return dim > 'financial'; }") + ); + validateSelectDimFilter( + builder, + new SelectorDimFilter(QueryRunnerTestHelper.qualityDimension, "financial", "GTE"), + new JavaScriptDimFilter(QueryRunnerTestHelper.qualityDimension, "function(dim){ return dim >= 'financial'; }") + ); + validateSelectDimFilter( + builder, + new SelectorDimFilter(QueryRunnerTestHelper.qualityDimension, "financial", "LT"), + new JavaScriptDimFilter(QueryRunnerTestHelper.qualityDimension, "function(dim){ return dim < 'financial'; }") + ); + validateSelectDimFilter( + builder, + new SelectorDimFilter(QueryRunnerTestHelper.qualityDimension, "financial", "LTE"), + new JavaScriptDimFilter(QueryRunnerTestHelper.qualityDimension, "function(dim){ return dim <= 'financial'; }") + ); + validateSelectDimFilter( + builder, + new SelectorDimFilter(QueryRunnerTestHelper.qualityDimension, "financial", "EQ"), + new JavaScriptDimFilter(QueryRunnerTestHelper.qualityDimension, "function(dim){ return dim === 'financial'; }") + ); + validateSelectDimFilter( + builder, + new SelectorDimFilter(QueryRunnerTestHelper.qualityDimension, "financial", "NE"), + new JavaScriptDimFilter(QueryRunnerTestHelper.qualityDimension, "function(dim){ return dim != 'financial'; }") + ); + + // null + validateSelectDimFilter( + builder, + new SelectorDimFilter("partial_null_column", "", "EQ"), + new JavaScriptDimFilter("partial_null_column", "function(dim){ return dim === null; }") + ); + validateSelectDimFilter( + builder, + new SelectorDimFilter("partial_null_column", "", "NE"), + new JavaScriptDimFilter("partial_null_column", "function(dim){ return dim != null; }") + ); + } + + private void validateSelectDimFilter( + Druids.SelectQueryBuilder builder, + SelectorDimFilter selectorFilter, + JavaScriptDimFilter javaScriptDimFilter + ) { + + Iterable> expected = Sequences.toList( + runner.run(builder.filters(javaScriptDimFilter).build(), ImmutableMap.of()), + Lists.>newArrayList() + ); + Iterable> results = Sequences.toList( + runner.run(builder.filters(selectorFilter).build(), ImmutableMap.of()), + Lists.>newArrayList() + ); + verify(expected, results); + } + @Test public void testSelectWithDimsAndMets() { @@ -615,7 +723,7 @@ private static void verify( Object actVal = acHolder.getEvent().get(ex.getKey()); // work around for current II limitations - if (acHolder.getEvent().get(ex.getKey()) instanceof Double) { + if (ex.getValue() instanceof Float && actVal instanceof Double) { actVal = ((Double) actVal).floatValue(); } Assert.assertEquals(ex.getValue(), actVal); diff --git a/processing/src/test/java/io/druid/query/timeseries/TimeseriesQueryRunnerTest.java b/processing/src/test/java/io/druid/query/timeseries/TimeseriesQueryRunnerTest.java index c3821630647b..785ea1e95eb0 100644 --- a/processing/src/test/java/io/druid/query/timeseries/TimeseriesQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/timeseries/TimeseriesQueryRunnerTest.java @@ -58,6 +58,7 @@ import java.io.IOException; import java.util.Arrays; import java.util.HashMap; +import java.util.Iterator; import java.util.List; import java.util.Map; @@ -2142,6 +2143,62 @@ public void testTimeSeriesWithFilteredAggInvertedNullValue() assertExpectedResults(expectedResults, actualResults); } + @Test + public void testTimeSeriesWithFilteredAggWithSelectDimFilter() + { + Druids.TimeseriesQueryBuilder builder = + Druids.newTimeseriesQueryBuilder() + .dataSource(QueryRunnerTestHelper.dataSource) + .granularity(QueryRunnerTestHelper.allGran) + .intervals(QueryRunnerTestHelper.firstToThird) + .descending(descending); + + // existing + validateSelectDimFilter(builder, QueryRunnerTestHelper.qualityDimension, ">", "entertainment", 20); + validateSelectDimFilter(builder, QueryRunnerTestHelper.qualityDimension, ">=", "entertainment", 22); + validateSelectDimFilter(builder, QueryRunnerTestHelper.qualityDimension, "<", "entertainment", 4); + validateSelectDimFilter(builder, QueryRunnerTestHelper.qualityDimension, "<=", "entertainment", 6); + validateSelectDimFilter(builder, QueryRunnerTestHelper.qualityDimension, "==", "entertainment", 2); + validateSelectDimFilter(builder, QueryRunnerTestHelper.qualityDimension, "<>", "entertainment", 24); + + // non-existing + validateSelectDimFilter(builder, QueryRunnerTestHelper.qualityDimension, ">", "financial", 20); + validateSelectDimFilter(builder, QueryRunnerTestHelper.qualityDimension, ">=", "financial", 20); + validateSelectDimFilter(builder, QueryRunnerTestHelper.qualityDimension, "<", "financial", 6); + validateSelectDimFilter(builder, QueryRunnerTestHelper.qualityDimension, "<=", "financial", 6); + validateSelectDimFilter(builder, QueryRunnerTestHelper.qualityDimension, "==", "financial", 0); + validateSelectDimFilter(builder, QueryRunnerTestHelper.qualityDimension, "<>", "financial", 26); + + // null + validateSelectDimFilter(builder, "partial_null_column", "==", "", 22); + validateSelectDimFilter(builder, "partial_null_column", "<>", "", 4); + } + + private void validateSelectDimFilter( + Druids.TimeseriesQueryBuilder builder, + String dimension, + String operation, + String value, + long expected + ) + { + builder.aggregators( + Arrays.asList( + new FilteredAggregatorFactory(new CountAggregatorFactory("filteredAgg"), + new SelectorDimFilter(dimension, value, operation) + ) + ) + ); + Iterable> results = Sequences.toList( + runner.run(builder.build(), ImmutableMap.of()), + Lists.>newArrayList() + ); + Iterator> iterator = results.iterator(); + Assert.assertTrue(iterator.hasNext()); + Assert.assertEquals(expected, iterator.next().getValue().getLongMetric("filteredAgg").longValue()); + Assert.assertFalse(iterator.hasNext()); + } + @Test public void testTimeseriesWithTimeColumn() { From da61469c980c11f3064e0bcf2d8798ec1df9609d Mon Sep 17 00:00:00 2001 From: "navis.ryu" Date: Wed, 17 Feb 2016 18:10:51 +0900 Subject: [PATCH 2/2] added more tests --- .../druid/segment/filter/SelectorFilter.java | 2 +- .../query/select/SelectQueryRunnerTest.java | 66 ++++++++++++++++++- .../timeseries/TimeseriesQueryRunnerTest.java | 25 +++++-- 3 files changed, 87 insertions(+), 6 deletions(-) diff --git a/processing/src/main/java/io/druid/segment/filter/SelectorFilter.java b/processing/src/main/java/io/druid/segment/filter/SelectorFilter.java index 4a59596492e8..7ce6f82b5cb4 100644 --- a/processing/src/main/java/io/druid/segment/filter/SelectorFilter.java +++ b/processing/src/main/java/io/druid/segment/filter/SelectorFilter.java @@ -109,7 +109,6 @@ public ValueMatcher makeMatcher(ColumnSelectorFactory columnSelectorFactory) new DefaultDimensionSpec(dimension, dimension) ); - // Missing columns match a null or empty string value and don't match anything else if (operator == BinaryOperator.EQ || operator == BinaryOperator.NE) { final ValueMatcher matcher = toValueMatcher(dimensionSelector); return operator == BinaryOperator.EQ ? matcher : new RevertedMatcher(matcher); @@ -136,6 +135,7 @@ public boolean matches() private ValueMatcher toValueMatcher(final DimensionSelector dimensionSelector) { if (dimensionSelector == null) { + // Missing columns match a null or empty string value and don't match anything else return new BooleanValueMatcher(Strings.isNullOrEmpty(value)); } final int valueId = dimensionSelector.lookupId(value); diff --git a/processing/src/test/java/io/druid/query/select/SelectQueryRunnerTest.java b/processing/src/test/java/io/druid/query/select/SelectQueryRunnerTest.java index 314bb95c9265..65aaf6151a9c 100644 --- a/processing/src/test/java/io/druid/query/select/SelectQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/select/SelectQueryRunnerTest.java @@ -329,7 +329,7 @@ public void testSelectWithSelectDimFilter() new JavaScriptDimFilter(QueryRunnerTestHelper.qualityDimension, "function(dim){ return dim != 'entertainment'; }") ); - // non-existing + // non-existing (between) validateSelectDimFilter( builder, new SelectorDimFilter(QueryRunnerTestHelper.qualityDimension, "financial", "GT"), @@ -361,6 +361,70 @@ public void testSelectWithSelectDimFilter() new JavaScriptDimFilter(QueryRunnerTestHelper.qualityDimension, "function(dim){ return dim != 'financial'; }") ); + // non-existing (smaller than min) + validateSelectDimFilter( + builder, + new SelectorDimFilter(QueryRunnerTestHelper.qualityDimension, "abcb", "GT"), + new JavaScriptDimFilter(QueryRunnerTestHelper.qualityDimension, "function(dim){ return dim > 'abcb'; }") + ); + validateSelectDimFilter( + builder, + new SelectorDimFilter(QueryRunnerTestHelper.qualityDimension, "abcb", "GTE"), + new JavaScriptDimFilter(QueryRunnerTestHelper.qualityDimension, "function(dim){ return dim >= 'abcb'; }") + ); + validateSelectDimFilter( + builder, + new SelectorDimFilter(QueryRunnerTestHelper.qualityDimension, "abcb", "LT"), + new JavaScriptDimFilter(QueryRunnerTestHelper.qualityDimension, "function(dim){ return dim < 'abcb'; }") + ); + validateSelectDimFilter( + builder, + new SelectorDimFilter(QueryRunnerTestHelper.qualityDimension, "abcb", "LTE"), + new JavaScriptDimFilter(QueryRunnerTestHelper.qualityDimension, "function(dim){ return dim <= 'abcb'; }") + ); + validateSelectDimFilter( + builder, + new SelectorDimFilter(QueryRunnerTestHelper.qualityDimension, "abcb", "EQ"), + new JavaScriptDimFilter(QueryRunnerTestHelper.qualityDimension, "function(dim){ return dim === 'abcb'; }") + ); + validateSelectDimFilter( + builder, + new SelectorDimFilter(QueryRunnerTestHelper.qualityDimension, "abcb", "NE"), + new JavaScriptDimFilter(QueryRunnerTestHelper.qualityDimension, "function(dim){ return dim != 'abcb'; }") + ); + + // non-existing (bigger than max) + validateSelectDimFilter( + builder, + new SelectorDimFilter(QueryRunnerTestHelper.qualityDimension, "zztop", "GT"), + new JavaScriptDimFilter(QueryRunnerTestHelper.qualityDimension, "function(dim){ return dim > 'zztop'; }") + ); + validateSelectDimFilter( + builder, + new SelectorDimFilter(QueryRunnerTestHelper.qualityDimension, "zztop", "GTE"), + new JavaScriptDimFilter(QueryRunnerTestHelper.qualityDimension, "function(dim){ return dim >= 'zztop'; }") + ); + validateSelectDimFilter( + builder, + new SelectorDimFilter(QueryRunnerTestHelper.qualityDimension, "zztop", "LT"), + new JavaScriptDimFilter(QueryRunnerTestHelper.qualityDimension, "function(dim){ return dim < 'zztop'; }") + ); + validateSelectDimFilter( + builder, + new SelectorDimFilter(QueryRunnerTestHelper.qualityDimension, "zztop", "LTE"), + new JavaScriptDimFilter(QueryRunnerTestHelper.qualityDimension, "function(dim){ return dim <= 'zztop'; }") + ); + validateSelectDimFilter( + builder, + new SelectorDimFilter(QueryRunnerTestHelper.qualityDimension, "zztop", "EQ"), + new JavaScriptDimFilter(QueryRunnerTestHelper.qualityDimension, "function(dim){ return dim === 'zztop'; }") + ); + validateSelectDimFilter( + builder, + new SelectorDimFilter(QueryRunnerTestHelper.qualityDimension, "zztop", "NE"), + new JavaScriptDimFilter(QueryRunnerTestHelper.qualityDimension, "function(dim){ return dim != 'zztop'; }") + ); + // null validateSelectDimFilter( builder, diff --git a/processing/src/test/java/io/druid/query/timeseries/TimeseriesQueryRunnerTest.java b/processing/src/test/java/io/druid/query/timeseries/TimeseriesQueryRunnerTest.java index 785ea1e95eb0..0c49e6befae1 100644 --- a/processing/src/test/java/io/druid/query/timeseries/TimeseriesQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/timeseries/TimeseriesQueryRunnerTest.java @@ -70,7 +70,7 @@ public class TimeseriesQueryRunnerTest public static final Map CONTEXT = ImmutableMap.of(); - @Parameterized.Parameters(name="{0}:descending={1}") + @Parameterized.Parameters(name = "{0}:descending={1}") public static Iterable constructorFeeder() throws IOException { return QueryRunnerTestHelper.cartesian( @@ -2161,7 +2161,7 @@ public void testTimeSeriesWithFilteredAggWithSelectDimFilter() validateSelectDimFilter(builder, QueryRunnerTestHelper.qualityDimension, "==", "entertainment", 2); validateSelectDimFilter(builder, QueryRunnerTestHelper.qualityDimension, "<>", "entertainment", 24); - // non-existing + // non-existing (between) validateSelectDimFilter(builder, QueryRunnerTestHelper.qualityDimension, ">", "financial", 20); validateSelectDimFilter(builder, QueryRunnerTestHelper.qualityDimension, ">=", "financial", 20); validateSelectDimFilter(builder, QueryRunnerTestHelper.qualityDimension, "<", "financial", 6); @@ -2169,6 +2169,22 @@ public void testTimeSeriesWithFilteredAggWithSelectDimFilter() validateSelectDimFilter(builder, QueryRunnerTestHelper.qualityDimension, "==", "financial", 0); validateSelectDimFilter(builder, QueryRunnerTestHelper.qualityDimension, "<>", "financial", 26); + // non-existing (smaller than min) + validateSelectDimFilter(builder, QueryRunnerTestHelper.qualityDimension, ">", "abcb", 26); + validateSelectDimFilter(builder, QueryRunnerTestHelper.qualityDimension, ">=", "abcb", 26); + validateSelectDimFilter(builder, QueryRunnerTestHelper.qualityDimension, "<", "abcb", 0); + validateSelectDimFilter(builder, QueryRunnerTestHelper.qualityDimension, "<=", "abcb", 0); + validateSelectDimFilter(builder, QueryRunnerTestHelper.qualityDimension, "==", "abcb", 0); + validateSelectDimFilter(builder, QueryRunnerTestHelper.qualityDimension, "<>", "abcb", 26); + + // non-existing (bigger than max) + validateSelectDimFilter(builder, QueryRunnerTestHelper.qualityDimension, ">", "zztop", 0); + validateSelectDimFilter(builder, QueryRunnerTestHelper.qualityDimension, ">=", "zztop", 0); + validateSelectDimFilter(builder, QueryRunnerTestHelper.qualityDimension, "<", "zztop", 26); + validateSelectDimFilter(builder, QueryRunnerTestHelper.qualityDimension, "<=", "zztop", 26); + validateSelectDimFilter(builder, QueryRunnerTestHelper.qualityDimension, "==", "zztop", 0); + validateSelectDimFilter(builder, QueryRunnerTestHelper.qualityDimension, "<>", "zztop", 26); + // null validateSelectDimFilter(builder, "partial_null_column", "==", "", 22); validateSelectDimFilter(builder, "partial_null_column", "<>", "", 4); @@ -2184,8 +2200,9 @@ private void validateSelectDimFilter( { builder.aggregators( Arrays.asList( - new FilteredAggregatorFactory(new CountAggregatorFactory("filteredAgg"), - new SelectorDimFilter(dimension, value, operation) + new FilteredAggregatorFactory( + new CountAggregatorFactory("filteredAgg"), + new SelectorDimFilter(dimension, value, operation) ) ) );