Merge pull request #2711 from gianm/filtered-aggregator-impls

All Filters should work with FilteredAggregators.
apache · Mar 23, 2016 · a5d5529 · a5d5529
2 parents a6af197 + dd86198
commit a5d5529
Show file tree

Hide file tree

Showing 15 changed files with 285 additions and 226 deletions.
diff --git a/docs/content/querying/aggregations.md b/docs/content/querying/aggregations.md
@@ -204,8 +204,6 @@ A filtered aggregator wraps any given aggregator, but only aggregates the values
 
 This makes it possible to compute the results of a filtered and an unfiltered aggregation simultaneously, without having to issue multiple queries, and use both results as part of post-aggregations.
 
-*Limitations:* The filtered aggregator currently only supports 'or', 'and', 'selector', 'not' and 'Extraction' filters, i.e. matching one or multiple dimensions against a single value.
-
 *Note:* If only the filtered results are required, consider putting the filter on the query itself, which will be much faster since it does not require scanning all the data.
 
 ```json

diff --git a/processing/src/main/java/io/druid/query/aggregation/FilteredAggregatorFactory.java b/processing/src/main/java/io/druid/query/aggregation/FilteredAggregatorFactory.java
@@ -21,12 +21,19 @@
 
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.google.common.base.Preconditions;
+import com.google.common.base.Predicate;
+import io.druid.query.dimension.DefaultDimensionSpec;
 import io.druid.query.filter.DimFilter;
 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.IndexedInts;
+import io.druid.segment.filter.BooleanValueMatcher;
 import io.druid.segment.filter.Filters;
 
 import java.nio.ByteBuffer;
+import java.util.BitSet;
 import java.util.Comparator;
 import java.util.List;
 
@@ -52,18 +59,19 @@ public FilteredAggregatorFactory(
   @Override
   public Aggregator factorize(ColumnSelectorFactory columnSelectorFactory)
   {
-    final ValueMatcher valueMatcher = Filters.convertDimensionFilters(filter).makeMatcher(columnSelectorFactory);
-      return new FilteredAggregator(
-          valueMatcher,
-          delegate.factorize(columnSelectorFactory)
-      );
-
+    final ValueMatcherFactory valueMatcherFactory = new FilteredAggregatorValueMatcherFactory(columnSelectorFactory);
+    final ValueMatcher valueMatcher = Filters.convertDimensionFilters(filter).makeMatcher(valueMatcherFactory);
+    return new FilteredAggregator(
+        valueMatcher,
+        delegate.factorize(columnSelectorFactory)
+    );
   }
 
   @Override
   public BufferAggregator factorizeBuffered(ColumnSelectorFactory columnSelectorFactory)
   {
-    final ValueMatcher valueMatcher = Filters.convertDimensionFilters(filter).makeMatcher(columnSelectorFactory);
+    final ValueMatcherFactory valueMatcherFactory = new FilteredAggregatorValueMatcherFactory(columnSelectorFactory);
+    final ValueMatcher valueMatcher = Filters.convertDimensionFilters(filter).makeMatcher(valueMatcherFactory);
     return new FilteredBufferAggregator(
         valueMatcher,
         delegate.factorizeBuffered(columnSelectorFactory)
@@ -199,4 +207,85 @@ public int hashCode()
     result = 31 * result + (filter != null ? filter.hashCode() : 0);
     return result;
   }
+
+  private static class FilteredAggregatorValueMatcherFactory implements ValueMatcherFactory
+  {
+    private final ColumnSelectorFactory columnSelectorFactory;
+
+    public FilteredAggregatorValueMatcherFactory(ColumnSelectorFactory columnSelectorFactory)
+    {
+      this.columnSelectorFactory = columnSelectorFactory;
+    }
+
+    @Override
+    public ValueMatcher makeValueMatcher(final String dimension, final Comparable value)
+    {
+      final DimensionSelector selector = columnSelectorFactory.makeDimensionSelector(
+          new DefaultDimensionSpec(dimension, dimension)
+      );
+
+      // Compare "value" as a String.
+      final String valueString = value == null ? null : value.toString();
+      final boolean isNullOrEmpty = valueString == null || valueString.isEmpty();
+
+      // Missing columns match a null or empty string value, and don't match anything else.
+      if (selector == null) {
+        return new BooleanValueMatcher(isNullOrEmpty);
+      }
+
+      final int valueId = selector.lookupId(valueString);
+      return new ValueMatcher()
+      {
+        @Override
+        public boolean matches()
+        {
+          final IndexedInts row = selector.getRow();
+          final int size = row.size();
+          for (int i = 0; i < size; ++i) {
+            if (row.get(i) == valueId) {
+              return true;
+            }
+          }
+          return false;
+        }
+      };
+    }
+
+    @Override
+    public ValueMatcher makeValueMatcher(final String dimension, final Predicate predicate)
+    {
+      final DimensionSelector selector = columnSelectorFactory.makeDimensionSelector(
+          new DefaultDimensionSpec(dimension, dimension)
+      );
+
+      if (selector == null) {
+        return new BooleanValueMatcher(predicate.apply(null));
+      }
+
+      // Check every value in the dimension, as a String.
+      final int cardinality = selector.getValueCardinality();
+      final BitSet valueIds = new BitSet(cardinality);
+      for (int i = 0; i < cardinality; i++) {
+        if (predicate.apply(selector.lookupName(i))) {
+          valueIds.set(i);
+        }
+      }
+
+      return new ValueMatcher()
+      {
+        @Override
+        public boolean matches()
+        {
+          final IndexedInts row = selector.getRow();
+          final int size = row.size();
+          for (int i = 0; i < size; ++i) {
+            if (valueIds.get(row.get(i))) {
+              return true;
+            }
+          }
+          return false;
+        }
+      };
+    }
+  }
 }
diff --git a/processing/src/main/java/io/druid/query/filter/Filter.java b/processing/src/main/java/io/druid/query/filter/Filter.java
@@ -20,13 +20,11 @@
 package io.druid.query.filter;
 
 import com.metamx.collections.bitmap.ImmutableBitmap;
-import io.druid.segment.ColumnSelectorFactory;
 
 /**
  */
 public interface Filter
 {
   public ImmutableBitmap getBitmapIndex(BitmapIndexSelector selector);
   public ValueMatcher makeMatcher(ValueMatcherFactory factory);
-  public ValueMatcher makeMatcher(ColumnSelectorFactory columnSelectorFactory);
 }
diff --git a/processing/src/main/java/io/druid/query/filter/ValueMatcherFactory.java b/processing/src/main/java/io/druid/query/filter/ValueMatcherFactory.java
@@ -20,13 +20,11 @@
 package io.druid.query.filter;
 
 import com.google.common.base.Predicate;
-import com.metamx.collections.spatial.search.Bound;
 
 /**
  */
 public interface ValueMatcherFactory
 {
   public ValueMatcher makeValueMatcher(String dimension, Comparable value);
-  public ValueMatcher makeValueMatcher(String dimension, Predicate value);
-  public ValueMatcher makeValueMatcher(String dimension, Bound bound);
+  public ValueMatcher makeValueMatcher(String dimension, Predicate predicate);
 }
diff --git a/processing/src/main/java/io/druid/segment/filter/AndFilter.java b/processing/src/main/java/io/druid/segment/filter/AndFilter.java
@@ -25,7 +25,6 @@
 import io.druid.query.filter.Filter;
 import io.druid.query.filter.ValueMatcher;
 import io.druid.query.filter.ValueMatcherFactory;
-import io.druid.segment.ColumnSelectorFactory;
 
 import java.util.List;
 
@@ -72,16 +71,6 @@ public ValueMatcher makeMatcher(ValueMatcherFactory factory)
     return makeMatcher(matchers);
   }
 
-  public ValueMatcher makeMatcher(ColumnSelectorFactory factory)
-  {
-    final ValueMatcher[] matchers = new ValueMatcher[filters.size()];
-
-    for (int i = 0; i < filters.size(); i++) {
-      matchers[i] = filters.get(i).makeMatcher(factory);
-    }
-    return makeMatcher(matchers);
-  }
-
   private ValueMatcher makeMatcher(final ValueMatcher[] baseMatchers)
   {
     if (baseMatchers.length == 1) {

diff --git a/processing/src/main/java/io/druid/segment/filter/DimensionPredicateFilter.java b/processing/src/main/java/io/druid/segment/filter/DimensionPredicateFilter.java
@@ -27,7 +27,6 @@
 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.data.Indexed;
 
 import javax.annotation.Nullable;
@@ -77,10 +76,4 @@ public ValueMatcher makeMatcher(ValueMatcherFactory factory)
   {
     return factory.makeValueMatcher(dimension, predicate);
   }
-
-  @Override
-  public ValueMatcher makeMatcher(ColumnSelectorFactory factory)
-  {
-    throw new UnsupportedOperationException();
-  }
 }
diff --git a/processing/src/main/java/io/druid/segment/filter/ExtractionFilter.java b/processing/src/main/java/io/druid/segment/filter/ExtractionFilter.java
@@ -23,18 +23,13 @@
 import com.google.common.base.Strings;
 import com.google.common.collect.Lists;
 import com.metamx.collections.bitmap.ImmutableBitmap;
-import io.druid.query.dimension.DefaultDimensionSpec;
 import io.druid.query.extraction.ExtractionFn;
 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;
 
-import java.util.BitSet;
 import java.util.Iterator;
 import java.util.List;
 
@@ -121,38 +116,4 @@ public boolean apply(String input)
         }
     );
   }
-
-  @Override
-  public ValueMatcher makeMatcher(ColumnSelectorFactory columnSelectorFactory)
-  {
-    final DimensionSelector dimensionSelector = columnSelectorFactory.makeDimensionSelector(
-        new DefaultDimensionSpec(dimension, dimension)
-    );
-    if (dimensionSelector == null) {
-      return new BooleanValueMatcher(value.equals(Strings.nullToEmpty(fn.apply(null))));
-    } else {
-      final BitSet bitSetOfIds = new BitSet(dimensionSelector.getValueCardinality());
-      for (int i = 0; i < dimensionSelector.getValueCardinality(); i++) {
-        if (value.equals(Strings.nullToEmpty(fn.apply(dimensionSelector.lookupName(i))))) {
-          bitSetOfIds.set(i);
-        }
-      }
-      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 (bitSetOfIds.get(row.get(i))) {
-              return true;
-            }
-          }
-          return false;
-        }
-      };
-    }
-  }
-
 }
diff --git a/processing/src/main/java/io/druid/segment/filter/JavaScriptFilter.java b/processing/src/main/java/io/druid/segment/filter/JavaScriptFilter.java
@@ -27,7 +27,6 @@
 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.data.Indexed;
 import org.mozilla.javascript.Context;
 import org.mozilla.javascript.Function;
@@ -161,11 +160,4 @@ public int hashCode()
       return script.hashCode();
     }
   }
-
-  @Override
-  public ValueMatcher makeMatcher(ColumnSelectorFactory factory)
-  {
-    throw new UnsupportedOperationException();
-  }
-
 }
diff --git a/processing/src/main/java/io/druid/segment/filter/NotFilter.java b/processing/src/main/java/io/druid/segment/filter/NotFilter.java
@@ -24,7 +24,6 @@
 import io.druid.query.filter.Filter;
 import io.druid.query.filter.ValueMatcher;
 import io.druid.query.filter.ValueMatcherFactory;
-import io.druid.segment.ColumnSelectorFactory;
 
 /**
  */
@@ -62,19 +61,4 @@ public boolean matches()
       }
     };
   }
-
-  @Override
-  public ValueMatcher makeMatcher(ColumnSelectorFactory factory)
-  {
-    final ValueMatcher baseMatcher = baseFilter.makeMatcher(factory);
-
-    return new ValueMatcher()
-    {
-      @Override
-      public boolean matches()
-      {
-        return !baseMatcher.matches();
-      }
-    };
-  }
 }
diff --git a/processing/src/main/java/io/druid/segment/filter/OrFilter.java b/processing/src/main/java/io/druid/segment/filter/OrFilter.java
@@ -25,7 +25,6 @@
 import io.druid.query.filter.Filter;
 import io.druid.query.filter.ValueMatcher;
 import io.druid.query.filter.ValueMatcherFactory;
-import io.druid.segment.ColumnSelectorFactory;
 
 import java.util.List;
 
@@ -72,16 +71,6 @@ public ValueMatcher makeMatcher(ValueMatcherFactory factory)
     return makeMatcher(matchers);
   }
 
-  public ValueMatcher makeMatcher(ColumnSelectorFactory factory)
-  {
-    final ValueMatcher[] matchers = new ValueMatcher[filters.size()];
-
-    for (int i = 0; i < filters.size(); i++) {
-      matchers[i] = filters.get(i).makeMatcher(factory);
-    }
-    return makeMatcher(matchers);
-  }
-
   private ValueMatcher makeMatcher(final ValueMatcher[] baseMatchers){
     if (baseMatchers.length == 1) {
       return baseMatchers[0];

diff --git a/processing/src/main/java/io/druid/segment/filter/SelectorFilter.java b/processing/src/main/java/io/druid/segment/filter/SelectorFilter.java
@@ -19,16 +19,11 @@
 
 package io.druid.segment.filter;
 
-import com.google.common.base.Strings;
 import com.metamx.collections.bitmap.ImmutableBitmap;
-import io.druid.query.dimension.DefaultDimensionSpec;
 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.IndexedInts;
 
 /**
  */
@@ -57,36 +52,4 @@ public ValueMatcher makeMatcher(ValueMatcherFactory factory)
   {
     return factory.makeValueMatcher(dimension, value);
   }
-
-  @Override
-  public ValueMatcher makeMatcher(ColumnSelectorFactory columnSelectorFactory)
-  {
-    final DimensionSelector dimensionSelector = columnSelectorFactory.makeDimensionSelector(
-        new DefaultDimensionSpec(dimension, dimension)
-    );
-
-    // Missing columns match a null or empty string value and don't match anything else
-    if (dimensionSelector == null) {
-      return new BooleanValueMatcher(Strings.isNullOrEmpty(value));
-    } else {
-      final int valueId = dimensionSelector.lookupId(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 (row.get(i) == valueId) {
-              return true;
-            }
-          }
-          return false;
-        }
-      };
-    }
-  }
-
-
 }