From dc24bceed136686c7a13400490d1723e375f3426 Mon Sep 17 00:00:00 2001 From: Keuntae Park Date: Mon, 4 Jan 2016 18:45:54 +0900 Subject: [PATCH 1/8] support multiple dimensions in JavaScriptDimFilter --- .../query/filter/JavaScriptDimFilter.java | 65 +++++++++-- .../query/filter/ValueMatcherFactory.java | 1 + .../java/io/druid/segment/filter/Filters.java | 2 +- .../segment/filter/JavaScriptFilter.java | 91 ++++++++++------ .../IncrementalIndexStorageAdapter.java | 58 ++++++++++ .../JavaScriptDimFilterSerDesrTest.java | 102 ++++++++++++++++++ .../query/filter/JavaScriptDimFilterTest.java | 14 ++- .../query/groupby/GroupByQueryRunnerTest.java | 40 ++++++- 8 files changed, 318 insertions(+), 55 deletions(-) create mode 100644 processing/src/test/java/io/druid/query/filter/JavaScriptDimFilterSerDesrTest.java diff --git a/processing/src/main/java/io/druid/query/filter/JavaScriptDimFilter.java b/processing/src/main/java/io/druid/query/filter/JavaScriptDimFilter.java index ed0df547bdd4..3f03c67fbb07 100644 --- a/processing/src/main/java/io/druid/query/filter/JavaScriptDimFilter.java +++ b/processing/src/main/java/io/druid/query/filter/JavaScriptDimFilter.java @@ -21,32 +21,47 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.base.Joiner; import com.google.common.base.Preconditions; import com.metamx.common.StringUtils; import java.nio.ByteBuffer; +import java.util.Arrays; public class JavaScriptDimFilter implements DimFilter { - private final String dimension; + private final String[] dimensions; private final String function; @JsonCreator public JavaScriptDimFilter( @JsonProperty("dimension") String dimension, + @JsonProperty("dimensions") String[] dimensions, @JsonProperty("function") String function ) { - Preconditions.checkArgument(dimension != null, "dimension must not be null"); + Preconditions.checkArgument(dimension != null || dimensions != null, "dimension or dimensions must not be null"); + Preconditions.checkArgument(!(dimension != null && dimensions != null), "both dimension and dimensions are defined"); Preconditions.checkArgument(function != null, "function must not be null"); - this.dimension = dimension; + this.dimensions = (dimension != null) ? new String[]{dimension} : dimensions; this.function = function; } @JsonProperty public String getDimension() { - return dimension; + return (dimensions.length == 1) ? dimensions[0] : null; + } + + @JsonProperty + public String[] getDimensions() + { + return (dimensions.length > 1) ? dimensions: null; + } + + public String[] getAllDimensions() + { + return dimensions; } @JsonProperty @@ -58,23 +73,51 @@ public String getFunction() @Override public byte[] getCacheKey() { - final byte[] dimensionBytes = StringUtils.toUtf8(dimension); final byte[] functionBytes = StringUtils.toUtf8(function); + byte[][] dimensionsBytes = new byte[dimensions.length][]; + int totalDimensionsBytes = 0; - return ByteBuffer.allocate(2 + dimensionBytes.length + functionBytes.length) - .put(DimFilterCacheHelper.JAVASCRIPT_CACHE_ID) - .put(dimensionBytes) - .put(DimFilterCacheHelper.STRING_SEPARATOR) - .put(functionBytes) + for (int idx = 0; idx < dimensions.length; idx++) { + dimensionsBytes[idx] = StringUtils.toUtf8(dimensions[idx]); + totalDimensionsBytes += dimensionsBytes[idx].length; + } + + ByteBuffer byteBuffer = ByteBuffer.allocate(2 + dimensions.length + totalDimensionsBytes + functionBytes.length) + .put(DimFilterCacheHelper.JAVASCRIPT_CACHE_ID); + for (byte[] dimBytes: dimensionsBytes) { + byteBuffer.put(dimBytes) + .put(DimFilterCacheHelper.STRING_SEPARATOR); + } + return byteBuffer.put(functionBytes) .array(); } @Override public String toString() { + String dimensionString = (dimensions.length == 1) ? "dimension='" + dimensions[0] + '\'' + : "dimensions=['" + Joiner.on("', '").join(dimensions) + "']"; return "JavaScriptDimFilter{" + - "dimension='" + dimension + '\'' + + dimensionString + ", function='" + function + '\'' + '}'; } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (!(o instanceof JavaScriptDimFilter)) { + return false; + } + + JavaScriptDimFilter that = (JavaScriptDimFilter) o; + + if (!function.equals(that.function)) { + return false; + } + return Arrays.equals(that.dimensions, dimensions); + } } diff --git a/processing/src/main/java/io/druid/query/filter/ValueMatcherFactory.java b/processing/src/main/java/io/druid/query/filter/ValueMatcherFactory.java index 8a2a52363eb6..f064d19073ce 100644 --- a/processing/src/main/java/io/druid/query/filter/ValueMatcherFactory.java +++ b/processing/src/main/java/io/druid/query/filter/ValueMatcherFactory.java @@ -28,5 +28,6 @@ public interface ValueMatcherFactory { public ValueMatcher makeValueMatcher(String dimension, String value); public ValueMatcher makeValueMatcher(String dimension, Predicate value); + public ValueMatcher makeValueMatcher(String[] dimensions, Predicate value); public ValueMatcher makeValueMatcher(String dimension, Bound bound); } 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..62e596e08265 100644 --- a/processing/src/main/java/io/druid/segment/filter/Filters.java +++ b/processing/src/main/java/io/druid/segment/filter/Filters.java @@ -93,7 +93,7 @@ public static Filter convertDimensionFilters(DimFilter dimFilter) } else if (dimFilter instanceof JavaScriptDimFilter) { final JavaScriptDimFilter javaScriptDimFilter = (JavaScriptDimFilter) dimFilter; - filter = new JavaScriptFilter(javaScriptDimFilter.getDimension(), javaScriptDimFilter.getFunction()); + filter = new JavaScriptFilter(javaScriptDimFilter.getAllDimensions(), javaScriptDimFilter.getFunction()); } else if (dimFilter instanceof SpatialDimFilter) { final SpatialDimFilter spatialDimFilter = (SpatialDimFilter) dimFilter; diff --git a/processing/src/main/java/io/druid/segment/filter/JavaScriptFilter.java b/processing/src/main/java/io/druid/segment/filter/JavaScriptFilter.java index 4ea9c4d04aa4..51394778a399 100644 --- a/processing/src/main/java/io/druid/segment/filter/JavaScriptFilter.java +++ b/processing/src/main/java/io/druid/segment/filter/JavaScriptFilter.java @@ -22,7 +22,6 @@ import com.google.common.base.Preconditions; import com.google.common.base.Predicate; import com.metamx.collections.bitmap.ImmutableBitmap; -import com.metamx.common.guava.FunctionalIterable; import io.druid.query.filter.BitmapIndexSelector; import io.druid.query.filter.Filter; import io.druid.query.filter.ValueMatcher; @@ -33,14 +32,14 @@ import org.mozilla.javascript.Function; import org.mozilla.javascript.ScriptableObject; -import javax.annotation.Nullable; +import java.util.Iterator; public class JavaScriptFilter implements Filter { private final JavaScriptPredicate predicate; - private final String dimension; + private final String[] dimension; - public JavaScriptFilter(String dimension, final String script) + public JavaScriptFilter(String[] dimension, final String script) { this.dimension = dimension; this.predicate = new JavaScriptPredicate(script); @@ -51,34 +50,56 @@ public ImmutableBitmap getBitmapIndex(final BitmapIndexSelector selector) { final Context cx = Context.enter(); try { - final Indexed dimValues = selector.getDimensionValues(dimension); ImmutableBitmap bitmap; - if (dimValues == null) { - bitmap = selector.getBitmapFactory().makeEmptyImmutableBitmap(); - } else { - bitmap = selector.getBitmapFactory().union( - FunctionalIterable.create(dimValues) - .filter( - new Predicate() - { - @Override - public boolean apply(@Nullable String input) - { - return predicate.applyInContext(cx, input); - } - } - ) - .transform( - new com.google.common.base.Function() - { - @Override - public ImmutableBitmap apply(@Nullable String input) - { - return selector.getBitmapIndex(dimension, input); - } - } - ) - ); + + boolean hasEmptyDimension = false; + Indexed [] dimValuesList = new Indexed[dimension.length]; + Iterator [] dimValuesIterator = new Iterator[dimension.length]; + String[] currentDim = new String[dimension.length]; + for(int idx = 0; idx < dimension.length; idx++) { + dimValuesList[idx] = selector.getDimensionValues(dimension[idx]); + if (dimValuesList[idx].size() == 0) { + hasEmptyDimension = true; + break; + } + dimValuesIterator[idx] = dimValuesList[idx].iterator(); + if (idx != 0) { + currentDim[idx] = dimValuesIterator[idx].next(); + } + } + + bitmap = selector.getBitmapFactory().makeEmptyImmutableBitmap(); + if (!hasEmptyDimension) { + int iteratingIndex = 0; + while(true) { + // advance iterator + Iterator iterator = dimValuesIterator[iteratingIndex]; + if (iterator.hasNext()) { + currentDim[iteratingIndex] = iterator.next(); + if (predicate.applyInContext(cx, currentDim)) + { + // update bitmap + ImmutableBitmap overlap = null; + for (int idx = 0; idx < dimension.length; idx++) { + ImmutableBitmap dimBitMap = selector.getBitmapIndex(dimension[idx], currentDim[idx]); + overlap = (overlap == null) ? dimBitMap : overlap.intersection(dimBitMap); + } + bitmap = bitmap.union(overlap); + } + + if (iteratingIndex > 0) { + // reset inner loop iterators + for (int idx = 0; idx < iteratingIndex; idx++) { + dimValuesIterator[idx] = dimValuesList[idx].iterator(); + } + // move to the most inner loop + iteratingIndex = 0; + } + } else { + iteratingIndex++; + if (iteratingIndex == dimension.length) break; + } + } } return bitmap; } @@ -94,7 +115,7 @@ public ValueMatcher makeMatcher(ValueMatcherFactory factory) return factory.makeValueMatcher(dimension, predicate); } - static class JavaScriptPredicate implements Predicate + static class JavaScriptPredicate implements Predicate { final ScriptableObject scope; final Function fnApply; @@ -118,7 +139,7 @@ public JavaScriptPredicate(final String script) } @Override - public boolean apply(final String input) + public boolean apply(final String[] input) { // one and only one context per thread final Context cx = Context.enter(); @@ -131,9 +152,9 @@ public boolean apply(final String input) } - public boolean applyInContext(Context cx, String input) + public boolean applyInContext(Context cx, String[] input) { - return Context.toBoolean(fnApply.call(cx, scope, scope, new String[]{input})); + return Context.toBoolean(fnApply.call(cx, scope, scope, input)); } @Override diff --git a/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexStorageAdapter.java b/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexStorageAdapter.java index 9f3b058e2b50..58e6207f4be0 100644 --- a/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexStorageAdapter.java +++ b/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexStorageAdapter.java @@ -684,6 +684,64 @@ public boolean matches() }; } + @Override + public ValueMatcher makeValueMatcher(final String[] dimensions, final Predicate predicate) + { + Integer[] dimIndexObject = new Integer[dimensions.length]; + final int[] dimIndex = new int[dimensions.length]; + for (int idx = 0; idx < dimensions.length; idx++) { + dimIndexObject[idx] = index.getDimensionIndex(dimensions[idx]); + if (dimIndexObject[idx] == null) { + return new BooleanValueMatcher(false); + } + dimIndex[idx] = dimIndexObject[idx]; + } + + return new ValueMatcher() + { + @Override + public boolean matches() + { + String[][] dims = holder.getKey().getDims(); + String[][] selected = new String[dimensions.length][]; + String[] current = new String[dimensions.length]; + int[] currentIdx = new int[dimensions.length]; + for (int idx = 0; idx < dimensions.length; idx++) { + if (dimIndex[idx] >= dims.length || dims[dimIndex[idx]] == null) { + return predicate.apply(null); + } + selected[idx] = dims[dimIndex[idx]]; + current[idx] = selected[idx][0]; + currentIdx[idx] = 0; + } + + if (predicate.apply(current)) { + return true; + } + + int currentIteratorIdx = 0; + while (true) { + currentIdx[currentIteratorIdx]++; + if (currentIdx[currentIteratorIdx] < selected[currentIteratorIdx].length) { + if (predicate.apply(current)) { + return true; + } + if (currentIteratorIdx > 0) { + for (int idx = 0; idx < currentIteratorIdx; idx++) { + currentIdx[idx] = 0; + } + currentIteratorIdx = 0; + } + } else { + currentIteratorIdx++; + if (currentIteratorIdx == dimensions.length) break; + } + } + return false; + } + }; + } + @Override public ValueMatcher makeValueMatcher(final String dimension, final Bound bound) { diff --git a/processing/src/test/java/io/druid/query/filter/JavaScriptDimFilterSerDesrTest.java b/processing/src/test/java/io/druid/query/filter/JavaScriptDimFilterSerDesrTest.java new file mode 100644 index 000000000000..9c817e6687c8 --- /dev/null +++ b/processing/src/test/java/io/druid/query/filter/JavaScriptDimFilterSerDesrTest.java @@ -0,0 +1,102 @@ +/* + * 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.query.filter; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.inject.Injector; +import com.google.inject.Key; +import io.druid.guice.GuiceInjectors; +import io.druid.guice.annotations.Json; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import java.io.IOException; +import java.util.Arrays; + +public class JavaScriptDimFilterSerDesrTest +{ + private static ObjectMapper mapper; + + private final String actualJSFilter1 = "{\"type\":\"javascript\",\"dimension\":\"dimTest\",\"dimensions\":null,\"function\":\"function(d1) { return true }\"}"; + private final String actualJSFilter2 = "{\"type\":\"javascript\",\"dimension\":null,\"dimensions\":[\"dimTest1\",\"dimTest2\"],\"function\":\"function(d1, d2) { return d1 === d2 }\"}"; + private final String actualJSFilterSkipNull1 = "{\"type\":\"javascript\",\"dimension\":\"dimTest\",\"function\":\"function(d1) { return true }\"}"; + private final String actualJSFilterSkipNull2 = "{\"type\":\"javascript\",\"dimensions\":[\"dimTest1\",\"dimTest2\"],\"function\":\"function(d1, d2) { return d1 === d2 }\"}"; + + @Before + public void setUp() + { + Injector defaultInjector = GuiceInjectors.makeStartupInjector(); + mapper = defaultInjector.getInstance(Key.get(ObjectMapper.class, Json.class)); + } + + @Test + public void testDeserialization1() throws IOException + { + final JavaScriptDimFilter actualJSDimFilter = mapper.reader(DimFilter.class).readValue(actualJSFilter1); + final JavaScriptDimFilter expectedJSDimFilter = + new JavaScriptDimFilter("dimTest", null, "function(d1) { return true }"); + Assert.assertEquals(expectedJSDimFilter, actualJSDimFilter); + } + + @Test + public void testDeserialization2() throws IOException + { + final JavaScriptDimFilter actualJSDimFilter = mapper.reader(DimFilter.class).readValue(actualJSFilter2); + final JavaScriptDimFilter expectedJSDimFilter = + new JavaScriptDimFilter(null, new String[]{"dimTest1", "dimTest2"}, "function(d1, d2) { return d1 === d2 }"); + Assert.assertEquals(expectedJSDimFilter, actualJSDimFilter); + } + + @Test + public void testDeserialization3() throws IOException + { + final JavaScriptDimFilter actualJSDimFilter = mapper.reader(DimFilter.class).readValue(actualJSFilterSkipNull1); + final JavaScriptDimFilter expectedJSDimFilter = + new JavaScriptDimFilter("dimTest", null, "function(d1) { return true }"); + Assert.assertEquals(expectedJSDimFilter, actualJSDimFilter); + } + + @Test + public void testDeserialization4() throws IOException + { + final JavaScriptDimFilter actualJSDimFilter = mapper.reader(DimFilter.class).readValue(actualJSFilterSkipNull2); + final JavaScriptDimFilter expectedJSDimFilter = + new JavaScriptDimFilter(null, new String[]{"dimTest1", "dimTest2"}, "function(d1, d2) { return d1 === d2 }"); + Assert.assertEquals(expectedJSDimFilter, actualJSDimFilter); + } + + @Test + public void testSerialization1() throws IOException + { + final JavaScriptDimFilter JSDimFilter = new JavaScriptDimFilter("dimTest", null, "function(d1) { return true }"); + final String expectedJSDimFilter = mapper.writeValueAsString(JSDimFilter); + Assert.assertEquals(expectedJSDimFilter, actualJSFilter1); + } + + @Test + public void testSerialization2() throws IOException + { + final JavaScriptDimFilter JSDimFilter = + new JavaScriptDimFilter(null, new String[]{"dimTest1", "dimTest2"}, "function(d1, d2) { return d1 === d2 }"); + final String expectedJSDimFilter = mapper.writeValueAsString(JSDimFilter); + Assert.assertEquals(expectedJSDimFilter, actualJSFilter2); + } +} diff --git a/processing/src/test/java/io/druid/query/filter/JavaScriptDimFilterTest.java b/processing/src/test/java/io/druid/query/filter/JavaScriptDimFilterTest.java index 3e8b37268743..b250f29f04c2 100644 --- a/processing/src/test/java/io/druid/query/filter/JavaScriptDimFilterTest.java +++ b/processing/src/test/java/io/druid/query/filter/JavaScriptDimFilterTest.java @@ -28,10 +28,18 @@ public class JavaScriptDimFilterTest { @Test - public void testGetCacheKey() + public void testGetCacheKey1() { - JavaScriptDimFilter javaScriptDimFilter = new JavaScriptDimFilter("dim", "fn"); - JavaScriptDimFilter javaScriptDimFilter2 = new JavaScriptDimFilter("di", "mfn"); + JavaScriptDimFilter javaScriptDimFilter = new JavaScriptDimFilter("dim", null, "fn"); + JavaScriptDimFilter javaScriptDimFilter2 = new JavaScriptDimFilter("di", null, "mfn"); + Assert.assertFalse(Arrays.equals(javaScriptDimFilter.getCacheKey(), javaScriptDimFilter2.getCacheKey())); + } + + @Test + public void testGetCacheKey2() + { + JavaScriptDimFilter javaScriptDimFilter = new JavaScriptDimFilter(null, new String[]{"dim1", "dim2"}, "fn"); + JavaScriptDimFilter javaScriptDimFilter2 = new JavaScriptDimFilter(null, new String[]{"dim1", "dim"}, "2fn"); Assert.assertFalse(Arrays.equals(javaScriptDimFilter.getCacheKey(), javaScriptDimFilter2.getCacheKey())); } } diff --git a/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java b/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java index 5d10abf97696..27c63256114f 100644 --- a/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java +++ b/processing/src/test/java/io/druid/query/groupby/GroupByQueryRunnerTest.java @@ -2348,7 +2348,7 @@ public void testIdenticalSubquery() .setDataSource(QueryRunnerTestHelper.dataSource) .setQuerySegmentSpec(QueryRunnerTestHelper.firstToThird) .setDimensions(Lists.newArrayList(new DefaultDimensionSpec("quality", "alias"))) - .setDimFilter(new JavaScriptDimFilter("quality", "function(dim){ return true; }")) + .setDimFilter(new JavaScriptDimFilter("quality", null, "function(dim){ return true; }")) .setAggregatorSpecs( Arrays.asList( QueryRunnerTestHelper.rowsCount, @@ -2407,7 +2407,7 @@ public void testSubqueryWithMultipleIntervalsInOuterQuery() .setDataSource(QueryRunnerTestHelper.dataSource) .setQuerySegmentSpec(QueryRunnerTestHelper.firstToThird) .setDimensions(Lists.newArrayList(new DefaultDimensionSpec("quality", "alias"))) - .setDimFilter(new JavaScriptDimFilter("quality", "function(dim){ return true; }")) + .setDimFilter(new JavaScriptDimFilter("quality", null, "function(dim){ return true; }")) .setAggregatorSpecs( Arrays.asList( QueryRunnerTestHelper.rowsCount, @@ -2690,7 +2690,7 @@ public void testSubqueryWithPostAggregators() .setDataSource(QueryRunnerTestHelper.dataSource) .setQuerySegmentSpec(QueryRunnerTestHelper.firstToThird) .setDimensions(Lists.newArrayList(new DefaultDimensionSpec("quality", "alias"))) - .setDimFilter(new JavaScriptDimFilter("quality", "function(dim){ return true; }")) + .setDimFilter(new JavaScriptDimFilter("quality", null, "function(dim){ return true; }")) .setAggregatorSpecs( Arrays.asList( QueryRunnerTestHelper.rowsCount, @@ -2951,7 +2951,7 @@ public void testSubqueryWithPostAggregatorsAndHaving() .setDataSource(QueryRunnerTestHelper.dataSource) .setQuerySegmentSpec(QueryRunnerTestHelper.firstToThird) .setDimensions(Lists.newArrayList(new DefaultDimensionSpec("quality", "alias"))) - .setDimFilter(new JavaScriptDimFilter("quality", "function(dim){ return true; }")) + .setDimFilter(new JavaScriptDimFilter("quality", null, "function(dim){ return true; }")) .setAggregatorSpecs( Arrays.asList( QueryRunnerTestHelper.rowsCount, @@ -3209,7 +3209,7 @@ public void testSubqueryWithMultiColumnAggregators() .setDataSource(QueryRunnerTestHelper.dataSource) .setQuerySegmentSpec(QueryRunnerTestHelper.firstToThird) .setDimensions(Lists.newArrayList(new DefaultDimensionSpec("quality", "alias"))) - .setDimFilter(new JavaScriptDimFilter("market", "function(dim){ return true; }")) + .setDimFilter(new JavaScriptDimFilter("market", null, "function(dim){ return true; }")) .setAggregatorSpecs( Arrays.asList( QueryRunnerTestHelper.rowsCount, @@ -4295,4 +4295,34 @@ public void testGroupByWithAggregatorFilterAndExtractionFunction() } + @Test + public void testJavaScriptDimFilterWithMultipleDimensions() + { + GroupByQuery query = GroupByQuery + .builder() + .setDataSource(QueryRunnerTestHelper.dataSource) + .setQuerySegmentSpec(QueryRunnerTestHelper.firstToThird) + .setDimensions(Lists.newArrayList(new DefaultDimensionSpec("market", "mkt"), new DefaultDimensionSpec("quality", "alias"))) + .setDimFilter(new JavaScriptDimFilter(null, new String[]{"market", "quality"}, "function(d1, d2){ return d1.length == d2.length; }")) + .setAggregatorSpecs( + Arrays.asList( + QueryRunnerTestHelper.rowsCount, + new LongSumAggregatorFactory("idx", "index") + ) + ) + .setGranularity(QueryRunnerTestHelper.dayGran) + .build(); + + List expectedResults = Arrays.asList( + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "alias", "news", "mkt", "spot", "rows", 1L, "idx", 121L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-01", "alias", "premium", "mkt", "upfront", "rows", 1L, "idx", 1234L), + + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-02", "alias", "news", "mkt", "spot", "rows", 1L, "idx", 114L), + GroupByQueryRunnerTestHelper.createExpectedRow("2011-04-02", "alias", "premium", "mkt", "upfront", "rows", 1L, "idx", 1049L) + ); + + Iterable results = GroupByQueryRunnerTestHelper.runQuery(factory, runner, query); + TestHelper.assertExpectedObjects(expectedResults, results, ""); + } + } From 98e8c10d745f134ad2b2559d2302aa33ace5747b Mon Sep 17 00:00:00 2001 From: Keuntae Park Date: Tue, 5 Jan 2016 11:44:35 +0900 Subject: [PATCH 2/8] add unit tests --- .../filter/JavaScriptDimFilterTest.java | 273 ++++++++++++++++++ 1 file changed, 273 insertions(+) create mode 100644 processing/src/test/java/io/druid/segment/filter/JavaScriptDimFilterTest.java diff --git a/processing/src/test/java/io/druid/segment/filter/JavaScriptDimFilterTest.java b/processing/src/test/java/io/druid/segment/filter/JavaScriptDimFilterTest.java new file mode 100644 index 000000000000..3832822f6c5a --- /dev/null +++ b/processing/src/test/java/io/druid/segment/filter/JavaScriptDimFilterTest.java @@ -0,0 +1,273 @@ +/* + * 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.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.metamx.collections.bitmap.BitmapFactory; +import com.metamx.collections.bitmap.ConciseBitmapFactory; +import com.metamx.collections.bitmap.ImmutableBitmap; +import com.metamx.collections.bitmap.MutableBitmap; +import com.metamx.collections.bitmap.RoaringBitmapFactory; +import com.metamx.collections.spatial.ImmutableRTree; +import io.druid.query.filter.BitmapIndexSelector; +import io.druid.query.filter.DimFilters; +import io.druid.query.filter.JavaScriptDimFilter; +import io.druid.segment.data.ArrayIndexed; +import io.druid.segment.data.Indexed; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +import java.util.LinkedHashMap; +import java.util.Map; + +/** + * + */ +@RunWith(Parameterized.class) +public class JavaScriptDimFilterTest +{ + private static final Map DIM_VALS = ImmutableMap.of( + "foo", new String[]{"foo1", "foo2", "foo3"}, + "bar", new String[]{"bar1", "foo3", "foo1"}, + "baz", new String[]{"foo1", "foo3", "foo3"} + ); + + @Parameterized.Parameters + public static Iterable constructorFeeder() + { + return ImmutableList.of( + new Object[]{new ConciseBitmapFactory()}, + new Object[]{new RoaringBitmapFactory()} + ); + } + + public JavaScriptDimFilterTest(BitmapFactory bitmapFactory) + { + bitmapMap = new LinkedHashMap<>(); + final MutableBitmap mutableBitmapfoo1 = bitmapFactory.makeEmptyMutableBitmap(); + final MutableBitmap mutableBitmapfoo2 = bitmapFactory.makeEmptyMutableBitmap(); + final MutableBitmap mutableBitmapfoo3 = bitmapFactory.makeEmptyMutableBitmap(); + mutableBitmapfoo1.add(0); + mutableBitmapfoo2.add(1); + mutableBitmapfoo3.add(2); + Map foo = new LinkedHashMap<>(); + foo.put("foo1", bitmapFactory.makeImmutableBitmap(mutableBitmapfoo1)); + foo.put("foo2", bitmapFactory.makeImmutableBitmap(mutableBitmapfoo2)); + foo.put("foo3", bitmapFactory.makeImmutableBitmap(mutableBitmapfoo3)); + this.bitmapMap.put("foo", foo); + + final MutableBitmap mutableBitmapbar1 = bitmapFactory.makeEmptyMutableBitmap(); + final MutableBitmap mutableBitmapbar2 = bitmapFactory.makeEmptyMutableBitmap(); + final MutableBitmap mutableBitmapbar3 = bitmapFactory.makeEmptyMutableBitmap(); + mutableBitmapbar1.add(0); + mutableBitmapbar2.add(1); + mutableBitmapbar3.add(2); + Map bar = new LinkedHashMap<>(); + bar.put("bar1", bitmapFactory.makeImmutableBitmap(mutableBitmapbar1)); + bar.put("foo3", bitmapFactory.makeImmutableBitmap(mutableBitmapbar2)); + bar.put("foo1", bitmapFactory.makeImmutableBitmap(mutableBitmapbar3)); + this.bitmapMap.put("bar", bar); + + final MutableBitmap mutableBitmapbaz1 = bitmapFactory.makeEmptyMutableBitmap(); + final MutableBitmap mutableBitmapbaz2 = bitmapFactory.makeEmptyMutableBitmap(); + final MutableBitmap mutableBitmapbaz3 = bitmapFactory.makeEmptyMutableBitmap(); + mutableBitmapbaz1.add(0); + mutableBitmapbaz2.add(1); + mutableBitmapbaz3.add(2); + Map baz = new LinkedHashMap<>(); + baz.put("foo1", bitmapFactory.makeImmutableBitmap(mutableBitmapbaz1)); + baz.put("foo3", bitmapFactory.makeImmutableBitmap(mutableBitmapbaz2)); + baz.put("foo3", bitmapFactory.makeImmutableBitmap(mutableBitmapbaz3)); + this.bitmapMap.put("baz", baz); + + this.factory = bitmapFactory; + } + + private final BitmapFactory factory; + private final Map> bitmapMap; + + private final BitmapIndexSelector BITMAP_INDEX_SELECTOR = new BitmapIndexSelector() + { + @Override + public Indexed getDimensionValues(String dimension) + { + final String[] vals = DIM_VALS.get(dimension); + return vals == null ? null : new ArrayIndexed(vals, String.class); + } + + @Override + public int getNumRows() + { + return 3; + } + + @Override + public BitmapFactory getBitmapFactory() + { + return factory; + } + + @Override + public ImmutableBitmap getBitmapIndex(String dimension, String value) + { + return bitmapMap.get(dimension).get(value); + } + + @Override + public ImmutableRTree getSpatialIndex(String dimension) + { + return null; + } + }; + private static final String javaScript = "function(dim1, dim2) { return dim1 === dim2 }"; + private static final String[] dimensions1 = {"foo", "bar"}; + private static final String[] dimensions2 = {"foo", "baz"}; + + @Test + public void testEmpty() + { + JavaScriptFilter javaScriptFilter = new JavaScriptFilter( + dimensions1, javaScript + ); + ImmutableBitmap immutableBitmap = javaScriptFilter.getBitmapIndex(BITMAP_INDEX_SELECTOR); + Assert.assertEquals(0, immutableBitmap.size()); + } + + @Test + public void testNormal() + { + JavaScriptFilter javaScriptFilter = new JavaScriptFilter( + dimensions2, javaScript + ); + ImmutableBitmap immutableBitmap = javaScriptFilter.getBitmapIndex(BITMAP_INDEX_SELECTOR); + Assert.assertEquals(2, immutableBitmap.size()); + } + + @Test + public void testOr() + { + Assert.assertEquals( + 2, Filters.convertDimensionFilters( + DimFilters.or( + new JavaScriptDimFilter( + null, + dimensions2, + javaScript + ) + ) + ).getBitmapIndex(BITMAP_INDEX_SELECTOR).size() + ); + + Assert.assertEquals( + 2, + Filters.convertDimensionFilters( + DimFilters.or( + new JavaScriptDimFilter( + null, + dimensions1, + javaScript + ), + new JavaScriptDimFilter( + null, + dimensions2, + javaScript + ) + ) + ).getBitmapIndex(BITMAP_INDEX_SELECTOR).size() + ); + } + + @Test + public void testAnd() + { + Assert.assertEquals( + 2, Filters.convertDimensionFilters( + DimFilters.or( + new JavaScriptDimFilter( + null, + dimensions2, + javaScript + ) + ) + ).getBitmapIndex(BITMAP_INDEX_SELECTOR).size() + ); + + Assert.assertEquals( + 0, + Filters.convertDimensionFilters( + DimFilters.and( + new JavaScriptDimFilter( + null, + dimensions1, + javaScript + ), + new JavaScriptDimFilter( + null, + dimensions2, + javaScript + ) + ) + ).getBitmapIndex(BITMAP_INDEX_SELECTOR).size() + ); + } + + @Test + public void testNot() + { + + Assert.assertEquals( + 2, Filters.convertDimensionFilters( + DimFilters.or( + new JavaScriptDimFilter( + null, + dimensions2, + javaScript + ) + ) + ).getBitmapIndex(BITMAP_INDEX_SELECTOR).size() + ); + + ImmutableBitmap result = Filters.convertDimensionFilters( + DimFilters.not( + new JavaScriptDimFilter( + null, + dimensions2, + javaScript + ) + ) + ).getBitmapIndex(BITMAP_INDEX_SELECTOR); + + Assert.assertEquals( + 1, + Filters.convertDimensionFilters( + DimFilters.not( + new JavaScriptDimFilter( + null, + dimensions2, + javaScript + ) + ) + ).getBitmapIndex(BITMAP_INDEX_SELECTOR).size() + ); + } +} From 24df1cd942a46adddd830492244270c250b68fcd Mon Sep 17 00:00:00 2001 From: Keuntae Park Date: Wed, 6 Jan 2016 17:44:52 +0900 Subject: [PATCH 3/8] update docs and fix test cases --- docs/content/querying/filters.md | 21 +++++++++++++++++-- .../filter/JavaScriptDimFilterTest.java | 13 +++--------- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/docs/content/querying/filters.md b/docs/content/querying/filters.md index 629151069f49..c8c5820cb83f 100644 --- a/docs/content/querying/filters.md +++ b/docs/content/querying/filters.md @@ -58,9 +58,9 @@ The filter specified at field can be any other filter defined on this page. ### JavaScript filter -The JavaScript filter matches a dimension against the specified JavaScript function predicate. The filter matches values for which the function returns true. +The JavaScript filter matches dimensions against the specified JavaScript function predicate. The filter matches values for which the function returns true. -The function takes a single argument, the dimension value, and returns either true or false. +The function takes the same number of arguments as the dimension values, and returns either true or false. ```json { @@ -69,6 +69,14 @@ The function takes a single argument, the dimension value, and returns either tr "function" : "function(value) { <...> }" } ``` +or +```json +{ + "type" : "javascript", + "dimensions" : , + "function" : "function(value1, value2, ...) { <...> }" +} +``` **Example** The following matches any dimension values for the dimension `name` between `'bar'` and `'foo'` @@ -81,6 +89,15 @@ The following matches any dimension values for the dimension `name` between `'ba } ``` +The following matches values of the given two dimensions `dim1` and `dim2` are the same +```json +{ + "type" : "javascript", + "dimension" : ["dim1","dim2"] + "function" : "function(x, y) { return x === y }" +} +``` + ### Extraction filter Extraction filter matches a dimension using some specific [Extraction function](./dimensionspecs.html#extraction-functions). diff --git a/processing/src/test/java/io/druid/segment/filter/JavaScriptDimFilterTest.java b/processing/src/test/java/io/druid/segment/filter/JavaScriptDimFilterTest.java index 3832822f6c5a..513eed68c4c3 100644 --- a/processing/src/test/java/io/druid/segment/filter/JavaScriptDimFilterTest.java +++ b/processing/src/test/java/io/druid/segment/filter/JavaScriptDimFilterTest.java @@ -231,6 +231,8 @@ public void testAnd() ); } + /* + * commented because complement() of roaring bitmap does not work correctly yet @Test public void testNot() { @@ -247,16 +249,6 @@ public void testNot() ).getBitmapIndex(BITMAP_INDEX_SELECTOR).size() ); - ImmutableBitmap result = Filters.convertDimensionFilters( - DimFilters.not( - new JavaScriptDimFilter( - null, - dimensions2, - javaScript - ) - ) - ).getBitmapIndex(BITMAP_INDEX_SELECTOR); - Assert.assertEquals( 1, Filters.convertDimensionFilters( @@ -270,4 +262,5 @@ public void testNot() ).getBitmapIndex(BITMAP_INDEX_SELECTOR).size() ); } + */ } From 82d873f5109c39c40e1dd53be8cca461e02181ce Mon Sep 17 00:00:00 2001 From: Keuntae Park Date: Wed, 6 Jan 2016 17:47:00 +0900 Subject: [PATCH 4/8] fix typo in docs --- docs/content/querying/filters.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/querying/filters.md b/docs/content/querying/filters.md index c8c5820cb83f..df36913908dd 100644 --- a/docs/content/querying/filters.md +++ b/docs/content/querying/filters.md @@ -93,7 +93,7 @@ The following matches values of the given two dimensions `dim1` and `dim2` are t ```json { "type" : "javascript", - "dimension" : ["dim1","dim2"] + "dimensions" : ["dim1","dim2"] "function" : "function(x, y) { return x === y }" } ``` From 55f25d8262b4fc956b4bc73626e79125e925d668 Mon Sep 17 00:00:00 2001 From: Keuntae Park Date: Wed, 6 Jan 2016 18:01:41 +0900 Subject: [PATCH 5/8] fix test cases and add some comments --- .../filter/JavaScriptDimFilterTest.java | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/processing/src/test/java/io/druid/segment/filter/JavaScriptDimFilterTest.java b/processing/src/test/java/io/druid/segment/filter/JavaScriptDimFilterTest.java index 513eed68c4c3..d4df60f91f5d 100644 --- a/processing/src/test/java/io/druid/segment/filter/JavaScriptDimFilterTest.java +++ b/processing/src/test/java/io/druid/segment/filter/JavaScriptDimFilterTest.java @@ -46,10 +46,19 @@ @RunWith(Parameterized.class) public class JavaScriptDimFilterTest { + /* + * 3 row values of three dimensions + * + * foo | bar | baz + * ==================== + * foo1 | bar1 | foo1 + * foo2 | foo3 | foo3 + * foo3 | foo1 | foo3 + */ private static final Map DIM_VALS = ImmutableMap.of( "foo", new String[]{"foo1", "foo2", "foo3"}, - "bar", new String[]{"bar1", "foo3", "foo1"}, - "baz", new String[]{"foo1", "foo3", "foo3"} + "bar", new String[]{"bar1", "foo1", "foo3"}, + "baz", new String[]{"foo1", "foo3"} ); @Parameterized.Parameters @@ -80,24 +89,22 @@ public JavaScriptDimFilterTest(BitmapFactory bitmapFactory) final MutableBitmap mutableBitmapbar2 = bitmapFactory.makeEmptyMutableBitmap(); final MutableBitmap mutableBitmapbar3 = bitmapFactory.makeEmptyMutableBitmap(); mutableBitmapbar1.add(0); - mutableBitmapbar2.add(1); - mutableBitmapbar3.add(2); + mutableBitmapbar2.add(2); + mutableBitmapbar3.add(1); Map bar = new LinkedHashMap<>(); bar.put("bar1", bitmapFactory.makeImmutableBitmap(mutableBitmapbar1)); - bar.put("foo3", bitmapFactory.makeImmutableBitmap(mutableBitmapbar2)); - bar.put("foo1", bitmapFactory.makeImmutableBitmap(mutableBitmapbar3)); + bar.put("foo1", bitmapFactory.makeImmutableBitmap(mutableBitmapbar2)); + bar.put("foo3", bitmapFactory.makeImmutableBitmap(mutableBitmapbar3)); this.bitmapMap.put("bar", bar); final MutableBitmap mutableBitmapbaz1 = bitmapFactory.makeEmptyMutableBitmap(); final MutableBitmap mutableBitmapbaz2 = bitmapFactory.makeEmptyMutableBitmap(); - final MutableBitmap mutableBitmapbaz3 = bitmapFactory.makeEmptyMutableBitmap(); mutableBitmapbaz1.add(0); mutableBitmapbaz2.add(1); - mutableBitmapbaz3.add(2); + mutableBitmapbaz2.add(2); Map baz = new LinkedHashMap<>(); baz.put("foo1", bitmapFactory.makeImmutableBitmap(mutableBitmapbaz1)); baz.put("foo3", bitmapFactory.makeImmutableBitmap(mutableBitmapbaz2)); - baz.put("foo3", bitmapFactory.makeImmutableBitmap(mutableBitmapbaz3)); this.bitmapMap.put("baz", baz); this.factory = bitmapFactory; From aa0df8a9b96ce558ea78d9ba02046b1c638185f5 Mon Sep 17 00:00:00 2001 From: Keuntae Park Date: Mon, 11 Jan 2016 11:15:42 +0900 Subject: [PATCH 6/8] updated based on review comments --- docs/content/querying/filters.md | 8 ---- .../query/filter/JavaScriptDimFilter.java | 17 ++------ .../java/io/druid/segment/filter/Filters.java | 2 +- .../JavaScriptDimFilterSerDesrTest.java | 40 +++++-------------- .../filter/JavaScriptDimFilterTest.java | 3 -- 5 files changed, 14 insertions(+), 56 deletions(-) diff --git a/docs/content/querying/filters.md b/docs/content/querying/filters.md index df36913908dd..c83b5dc36d7e 100644 --- a/docs/content/querying/filters.md +++ b/docs/content/querying/filters.md @@ -62,14 +62,6 @@ The JavaScript filter matches dimensions against the specified JavaScript functi The function takes the same number of arguments as the dimension values, and returns either true or false. -```json -{ - "type" : "javascript", - "dimension" : , - "function" : "function(value) { <...> }" -} -``` -or ```json { "type" : "javascript", diff --git a/processing/src/main/java/io/druid/query/filter/JavaScriptDimFilter.java b/processing/src/main/java/io/druid/query/filter/JavaScriptDimFilter.java index 3b27047327b8..295392ded870 100644 --- a/processing/src/main/java/io/druid/query/filter/JavaScriptDimFilter.java +++ b/processing/src/main/java/io/druid/query/filter/JavaScriptDimFilter.java @@ -35,31 +35,20 @@ public class JavaScriptDimFilter implements DimFilter @JsonCreator public JavaScriptDimFilter( + // for backwards compatibility @JsonProperty("dimension") String dimension, @JsonProperty("dimensions") String[] dimensions, @JsonProperty("function") String function ) { - Preconditions.checkArgument(dimension != null || dimensions != null, "dimension or dimensions must not be null"); - Preconditions.checkArgument(!(dimension != null && dimensions != null), "both dimension and dimensions are defined"); + Preconditions.checkArgument(dimension != null ^ dimensions != null, "dimensions(xor dimension) must not be null"); Preconditions.checkArgument(function != null, "function must not be null"); - this.dimensions = (dimension != null) ? new String[]{dimension} : dimensions; + this.dimensions = (dimensions != null) ? dimensions : new String[]{dimension}; this.function = function; } - @JsonProperty - public String getDimension() - { - return (dimensions.length == 1) ? dimensions[0] : null; - } - @JsonProperty public String[] getDimensions() - { - return (dimensions.length > 1) ? dimensions: null; - } - - public String[] getAllDimensions() { return dimensions; } 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 62e596e08265..796f0b3df68b 100644 --- a/processing/src/main/java/io/druid/segment/filter/Filters.java +++ b/processing/src/main/java/io/druid/segment/filter/Filters.java @@ -93,7 +93,7 @@ public static Filter convertDimensionFilters(DimFilter dimFilter) } else if (dimFilter instanceof JavaScriptDimFilter) { final JavaScriptDimFilter javaScriptDimFilter = (JavaScriptDimFilter) dimFilter; - filter = new JavaScriptFilter(javaScriptDimFilter.getAllDimensions(), javaScriptDimFilter.getFunction()); + filter = new JavaScriptFilter(javaScriptDimFilter.getDimensions(), javaScriptDimFilter.getFunction()); } else if (dimFilter instanceof SpatialDimFilter) { final SpatialDimFilter spatialDimFilter = (SpatialDimFilter) dimFilter; diff --git a/processing/src/test/java/io/druid/query/filter/JavaScriptDimFilterSerDesrTest.java b/processing/src/test/java/io/druid/query/filter/JavaScriptDimFilterSerDesrTest.java index 9c817e6687c8..4cc8e9afa07c 100644 --- a/processing/src/test/java/io/druid/query/filter/JavaScriptDimFilterSerDesrTest.java +++ b/processing/src/test/java/io/druid/query/filter/JavaScriptDimFilterSerDesrTest.java @@ -29,16 +29,14 @@ import org.junit.Test; import java.io.IOException; -import java.util.Arrays; public class JavaScriptDimFilterSerDesrTest { private static ObjectMapper mapper; - private final String actualJSFilter1 = "{\"type\":\"javascript\",\"dimension\":\"dimTest\",\"dimensions\":null,\"function\":\"function(d1) { return true }\"}"; - private final String actualJSFilter2 = "{\"type\":\"javascript\",\"dimension\":null,\"dimensions\":[\"dimTest1\",\"dimTest2\"],\"function\":\"function(d1, d2) { return d1 === d2 }\"}"; - private final String actualJSFilterSkipNull1 = "{\"type\":\"javascript\",\"dimension\":\"dimTest\",\"function\":\"function(d1) { return true }\"}"; - private final String actualJSFilterSkipNull2 = "{\"type\":\"javascript\",\"dimensions\":[\"dimTest1\",\"dimTest2\"],\"function\":\"function(d1, d2) { return d1 === d2 }\"}"; + private final String JSFilter = "{\"type\":\"javascript\",\"dimensions\":[\"dimTest1\",\"dimTest2\"],\"function\":\"function(d1, d2) { return d1 === d2 }\"}"; + private final String JSFilterBackwardCompatible = "{\"type\":\"javascript\",\"dimension\":\"dimTest\",\"function\":\"function(d1) { return true }\"}"; + private final String JSFilterRewritten = "{\"type\":\"javascript\",\"dimensions\":[\"dimTest\"],\"function\":\"function(d1) { return true }\"}"; @Before public void setUp() @@ -50,7 +48,7 @@ public void setUp() @Test public void testDeserialization1() throws IOException { - final JavaScriptDimFilter actualJSDimFilter = mapper.reader(DimFilter.class).readValue(actualJSFilter1); + final JavaScriptDimFilter actualJSDimFilter = mapper.reader(DimFilter.class).readValue(JSFilterBackwardCompatible); final JavaScriptDimFilter expectedJSDimFilter = new JavaScriptDimFilter("dimTest", null, "function(d1) { return true }"); Assert.assertEquals(expectedJSDimFilter, actualJSDimFilter); @@ -59,25 +57,7 @@ public void testDeserialization1() throws IOException @Test public void testDeserialization2() throws IOException { - final JavaScriptDimFilter actualJSDimFilter = mapper.reader(DimFilter.class).readValue(actualJSFilter2); - final JavaScriptDimFilter expectedJSDimFilter = - new JavaScriptDimFilter(null, new String[]{"dimTest1", "dimTest2"}, "function(d1, d2) { return d1 === d2 }"); - Assert.assertEquals(expectedJSDimFilter, actualJSDimFilter); - } - - @Test - public void testDeserialization3() throws IOException - { - final JavaScriptDimFilter actualJSDimFilter = mapper.reader(DimFilter.class).readValue(actualJSFilterSkipNull1); - final JavaScriptDimFilter expectedJSDimFilter = - new JavaScriptDimFilter("dimTest", null, "function(d1) { return true }"); - Assert.assertEquals(expectedJSDimFilter, actualJSDimFilter); - } - - @Test - public void testDeserialization4() throws IOException - { - final JavaScriptDimFilter actualJSDimFilter = mapper.reader(DimFilter.class).readValue(actualJSFilterSkipNull2); + final JavaScriptDimFilter actualJSDimFilter = mapper.reader(DimFilter.class).readValue(JSFilter); final JavaScriptDimFilter expectedJSDimFilter = new JavaScriptDimFilter(null, new String[]{"dimTest1", "dimTest2"}, "function(d1, d2) { return d1 === d2 }"); Assert.assertEquals(expectedJSDimFilter, actualJSDimFilter); @@ -86,17 +66,17 @@ public void testDeserialization4() throws IOException @Test public void testSerialization1() throws IOException { - final JavaScriptDimFilter JSDimFilter = new JavaScriptDimFilter("dimTest", null, "function(d1) { return true }"); + final JavaScriptDimFilter JSDimFilter = + new JavaScriptDimFilter(null, new String[]{"dimTest1", "dimTest2"}, "function(d1, d2) { return d1 === d2 }"); final String expectedJSDimFilter = mapper.writeValueAsString(JSDimFilter); - Assert.assertEquals(expectedJSDimFilter, actualJSFilter1); + Assert.assertEquals(expectedJSDimFilter, JSFilter); } @Test public void testSerialization2() throws IOException { - final JavaScriptDimFilter JSDimFilter = - new JavaScriptDimFilter(null, new String[]{"dimTest1", "dimTest2"}, "function(d1, d2) { return d1 === d2 }"); + final JavaScriptDimFilter JSDimFilter = new JavaScriptDimFilter("dimTest", null, "function(d1) { return true }"); final String expectedJSDimFilter = mapper.writeValueAsString(JSDimFilter); - Assert.assertEquals(expectedJSDimFilter, actualJSFilter2); + Assert.assertEquals(expectedJSDimFilter, JSFilterRewritten); } } diff --git a/processing/src/test/java/io/druid/segment/filter/JavaScriptDimFilterTest.java b/processing/src/test/java/io/druid/segment/filter/JavaScriptDimFilterTest.java index d4df60f91f5d..904c0f464ab2 100644 --- a/processing/src/test/java/io/druid/segment/filter/JavaScriptDimFilterTest.java +++ b/processing/src/test/java/io/druid/segment/filter/JavaScriptDimFilterTest.java @@ -238,8 +238,6 @@ public void testAnd() ); } - /* - * commented because complement() of roaring bitmap does not work correctly yet @Test public void testNot() { @@ -269,5 +267,4 @@ public void testNot() ).getBitmapIndex(BITMAP_INDEX_SELECTOR).size() ); } - */ } From f0498cc89c5279f8093ea637f3bc48ce79846956 Mon Sep 17 00:00:00 2001 From: Keuntae Park Date: Tue, 12 Jan 2016 15:07:23 +0900 Subject: [PATCH 7/8] bug fix on getBitmapIndex() of JavaScriptFilter --- .../query/filter/JavaScriptDimFilter.java | 4 +--- .../segment/filter/JavaScriptFilter.java | 22 +++++++++---------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/processing/src/main/java/io/druid/query/filter/JavaScriptDimFilter.java b/processing/src/main/java/io/druid/query/filter/JavaScriptDimFilter.java index 295392ded870..efc847e2f8d4 100644 --- a/processing/src/main/java/io/druid/query/filter/JavaScriptDimFilter.java +++ b/processing/src/main/java/io/druid/query/filter/JavaScriptDimFilter.java @@ -90,10 +90,8 @@ public DimFilter optimize() @Override public String toString() { - String dimensionString = (dimensions.length == 1) ? "dimension='" + dimensions[0] + '\'' - : "dimensions=['" + Joiner.on("', '").join(dimensions) + "']"; return "JavaScriptDimFilter{" + - dimensionString + + "dimensions=['" + Joiner.on("', '").join(dimensions) + "']" + ", function='" + function + '\'' + '}'; } diff --git a/processing/src/main/java/io/druid/segment/filter/JavaScriptFilter.java b/processing/src/main/java/io/druid/segment/filter/JavaScriptFilter.java index 51394778a399..a0966813dc50 100644 --- a/processing/src/main/java/io/druid/segment/filter/JavaScriptFilter.java +++ b/processing/src/main/java/io/druid/segment/filter/JavaScriptFilter.java @@ -76,18 +76,18 @@ public ImmutableBitmap getBitmapIndex(final BitmapIndexSelector selector) Iterator iterator = dimValuesIterator[iteratingIndex]; if (iterator.hasNext()) { currentDim[iteratingIndex] = iterator.next(); - if (predicate.applyInContext(cx, currentDim)) - { - // update bitmap - ImmutableBitmap overlap = null; - for (int idx = 0; idx < dimension.length; idx++) { - ImmutableBitmap dimBitMap = selector.getBitmapIndex(dimension[idx], currentDim[idx]); - overlap = (overlap == null) ? dimBitMap : overlap.intersection(dimBitMap); + if (iteratingIndex == 0) { + if (predicate.applyInContext(cx, currentDim)) + { + // update bitmap + ImmutableBitmap overlap = null; + for (int idx = 0; idx < dimension.length; idx++) { + ImmutableBitmap dimBitMap = selector.getBitmapIndex(dimension[idx], currentDim[idx]); + overlap = (overlap == null) ? dimBitMap : overlap.intersection(dimBitMap); + } + bitmap = bitmap.union(overlap); } - bitmap = bitmap.union(overlap); - } - - if (iteratingIndex > 0) { + } else { // reset inner loop iterators for (int idx = 0; idx < iteratingIndex; idx++) { dimValuesIterator[idx] = dimValuesList[idx].iterator(); From 4d205b86a8a07dbed153e9ecae49e5f07baedff5 Mon Sep 17 00:00:00 2001 From: Keuntae Park Date: Tue, 12 Jan 2016 16:34:16 +0900 Subject: [PATCH 8/8] fix bugs on cartesian product loop --- .../segment/filter/JavaScriptFilter.java | 25 ++++++++++--------- .../IncrementalIndexStorageAdapter.java | 8 +++--- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/processing/src/main/java/io/druid/segment/filter/JavaScriptFilter.java b/processing/src/main/java/io/druid/segment/filter/JavaScriptFilter.java index a0966813dc50..2bb2a3e87910 100644 --- a/processing/src/main/java/io/druid/segment/filter/JavaScriptFilter.java +++ b/processing/src/main/java/io/druid/segment/filter/JavaScriptFilter.java @@ -76,25 +76,26 @@ public ImmutableBitmap getBitmapIndex(final BitmapIndexSelector selector) Iterator iterator = dimValuesIterator[iteratingIndex]; if (iterator.hasNext()) { currentDim[iteratingIndex] = iterator.next(); - if (iteratingIndex == 0) { - if (predicate.applyInContext(cx, currentDim)) - { - // update bitmap - ImmutableBitmap overlap = null; - for (int idx = 0; idx < dimension.length; idx++) { - ImmutableBitmap dimBitMap = selector.getBitmapIndex(dimension[idx], currentDim[idx]); - overlap = (overlap == null) ? dimBitMap : overlap.intersection(dimBitMap); - } - bitmap = bitmap.union(overlap); - } - } else { + if (iteratingIndex > 0) { // reset inner loop iterators for (int idx = 0; idx < iteratingIndex; idx++) { dimValuesIterator[idx] = dimValuesList[idx].iterator(); + currentDim[idx] = dimValuesIterator[idx].next(); } // move to the most inner loop iteratingIndex = 0; } + if (predicate.applyInContext(cx, currentDim)) + { + // update bitmap + ImmutableBitmap overlap = null; + for (int idx = 0; idx < dimension.length; idx++) { + ImmutableBitmap dimBitMap = selector.getBitmapIndex(dimension[idx], currentDim[idx]); + overlap = (overlap == null) ? dimBitMap : overlap.intersection(dimBitMap); + } + bitmap = bitmap.union(overlap); + } + } else { iteratingIndex++; if (iteratingIndex == dimension.length) break; diff --git a/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexStorageAdapter.java b/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexStorageAdapter.java index 9f0ae619a007..fec927bce0ca 100644 --- a/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexStorageAdapter.java +++ b/processing/src/main/java/io/druid/segment/incremental/IncrementalIndexStorageAdapter.java @@ -723,15 +723,17 @@ public boolean matches() while (true) { currentIdx[currentIteratorIdx]++; if (currentIdx[currentIteratorIdx] < selected[currentIteratorIdx].length) { - if (predicate.apply(current)) { - return true; - } + current[currentIteratorIdx] = selected[currentIteratorIdx][currentIdx[currentIteratorIdx]]; if (currentIteratorIdx > 0) { for (int idx = 0; idx < currentIteratorIdx; idx++) { currentIdx[idx] = 0; + current[idx] = selected[idx][0]; } currentIteratorIdx = 0; } + if (predicate.apply(current)) { + return true; + } } else { currentIteratorIdx++; if (currentIteratorIdx == dimensions.length) break;