Filters: Use ColumnSelectorFactory directly for building row-based matchers.

common/src/main/java/io/druid/common/guava/SettableSupplier.java

@@ -0,0 +1,50 @@
+/*
+ * 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.common.guava;
+
+import com.google.common.base.Supplier;
+
+/**
+ * A settable Supplier. Not thread safe.
+ */
+public class SettableSupplier<T> implements Supplier<T>
+{
+  private T obj;
+
+  public SettableSupplier()
+  {
+  }
+
+  public SettableSupplier(T initialValue)
+  {
+    obj = initialValue;
+  }
+
+  public void set(T obj)
+  {
+    this.obj = obj;
+  }
+
+  @Override
+  public T get()
+  {
+    return obj;
+  }
+}

indexing-hadoop/src/main/java/io/druid/indexer/InputRowSerde.java

@@ -89,8 +89,7 @@ public InputRow get()
             IncrementalIndex.makeColumnSelectorFactory(
                 aggFactory,
                 supplier,
-                true,
-                null
+                true
             )
         );
         try {

processing/src/main/java/io/druid/query/aggregation/FilteredAggregatorFactory.java

@@ -21,23 +21,9 @@
 
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableList;
-import io.druid.query.ColumnSelectorPlus;
-import io.druid.query.dimension.DefaultDimensionSpec;
-import io.druid.query.dimension.DimensionSpec;
 import io.druid.query.filter.DimFilter;
-import io.druid.query.filter.DruidLongPredicate;
-import io.druid.query.filter.DruidPredicateFactory;
 import io.druid.query.filter.ValueMatcher;
-import io.druid.query.filter.ValueMatcherColumnSelectorStrategy;
-import io.druid.query.filter.ValueMatcherColumnSelectorStrategyFactory;
-import io.druid.query.filter.ValueMatcherFactory;
 import io.druid.segment.ColumnSelectorFactory;
-import io.druid.segment.DimensionHandlerUtils;
-import io.druid.segment.column.Column;
-import io.druid.segment.column.ColumnCapabilities;
-import io.druid.segment.column.ValueType;
-import io.druid.segment.filter.BooleanValueMatcher;
 import io.druid.segment.filter.Filters;
 
 import java.nio.ByteBuffer;
@@ -66,8 +52,7 @@ public FilteredAggregatorFactory(
   @Override
   public Aggregator factorize(ColumnSelectorFactory columnSelectorFactory)
   {
-    final ValueMatcherFactory valueMatcherFactory = new FilteredAggregatorValueMatcherFactory(columnSelectorFactory);
-    final ValueMatcher valueMatcher = Filters.toFilter(filter).makeMatcher(valueMatcherFactory);
+    final ValueMatcher valueMatcher = Filters.toFilter(filter).makeMatcher(columnSelectorFactory);
     return new FilteredAggregator(
         valueMatcher,
         delegate.factorize(columnSelectorFactory)
@@ -77,8 +62,7 @@ public Aggregator factorize(ColumnSelectorFactory columnSelectorFactory)
   @Override
   public BufferAggregator factorizeBuffered(ColumnSelectorFactory columnSelectorFactory)
   {
-    final ValueMatcherFactory valueMatcherFactory = new FilteredAggregatorValueMatcherFactory(columnSelectorFactory);
-    final ValueMatcher valueMatcher = Filters.toFilter(filter).makeMatcher(valueMatcherFactory);
+    final ValueMatcher valueMatcher = Filters.toFilter(filter).makeMatcher(columnSelectorFactory);
     return new FilteredBufferAggregator(
         valueMatcher,
         delegate.factorizeBuffered(columnSelectorFactory)
@@ -208,81 +192,4 @@ public int hashCode()
     result = 31 * result + (filter != null ? filter.hashCode() : 0);
     return result;
   }
-
-  private static class FilteredAggregatorValueMatcherFactory implements ValueMatcherFactory
-  {
-    private static final ValueMatcherColumnSelectorStrategyFactory STRATEGY_FACTORY =
-        new ValueMatcherColumnSelectorStrategyFactory();
-
-    private final ColumnSelectorFactory columnSelectorFactory;
-
-    public FilteredAggregatorValueMatcherFactory(ColumnSelectorFactory columnSelectorFactory)
-    {
-      this.columnSelectorFactory = columnSelectorFactory;
-    }
-
-    @Override
-    public ValueMatcher makeValueMatcher(final String dimension, final String value)
-    {
-      if (getTypeForDimension(dimension) == ValueType.LONG) {
-        return Filters.getLongValueMatcher(
-            columnSelectorFactory.makeLongColumnSelector(dimension),
-            value
-        );
-      }
-
-      ColumnSelectorPlus<ValueMatcherColumnSelectorStrategy>[] selector =
-          DimensionHandlerUtils.createColumnSelectorPluses(
-              STRATEGY_FACTORY,
-              ImmutableList.<DimensionSpec>of(DefaultDimensionSpec.of(dimension)),
-              columnSelectorFactory
-          );
-
-
-      final ValueMatcherColumnSelectorStrategy strategy = selector[0].getColumnSelectorStrategy();
-      return strategy.getValueMatcher(dimension, columnSelectorFactory, value);
-    }
-
-    public ValueMatcher makeValueMatcher(final String dimension, final DruidPredicateFactory predicateFactory)
-    {
-      ValueType type = getTypeForDimension(dimension);
-      switch (type) {
-        case LONG:
-          return makeLongValueMatcher(dimension, predicateFactory.makeLongPredicate());
-        case STRING:
-          ColumnSelectorPlus<ValueMatcherColumnSelectorStrategy>[] selector =
-              DimensionHandlerUtils.createColumnSelectorPluses(
-                  STRATEGY_FACTORY,
-                  ImmutableList.<DimensionSpec>of(DefaultDimensionSpec.of(dimension)),
-                  columnSelectorFactory
-              );
-
-
-          final ValueMatcherColumnSelectorStrategy strategy = selector[0].getColumnSelectorStrategy();
-          return strategy.getValueMatcher(dimension, columnSelectorFactory, predicateFactory);
-        default:
-          return new BooleanValueMatcher(predicateFactory.makeStringPredicate().apply(null));
-      }
-    }
-
-    private ValueMatcher makeLongValueMatcher(String dimension, DruidLongPredicate predicate)
-    {
-      return Filters.getLongPredicateMatcher(
-          columnSelectorFactory.makeLongColumnSelector(dimension),
-          predicate
-      );
-    }
-
-    private ValueType getTypeForDimension(String dimension)
-    {
-      // FilteredAggregatorFactory is sometimes created from a ColumnSelectorFactory that
-      // has no knowledge of column capabilities/types.
-      // Default to LONG for __time, STRING for everything else.
-      if (dimension.equals(Column.TIME_COLUMN_NAME)) {
-        return ValueType.LONG;
-      }
-      ColumnCapabilities capabilities = columnSelectorFactory.getColumnCapabilities(dimension);
-      return capabilities == null ? ValueType.STRING : capabilities.getType();
-    }
-  }
 }

processing/src/main/java/io/druid/query/filter/BooleanFilter.java

@@ -19,6 +19,8 @@
 
 package io.druid.query.filter;
 
+import io.druid.segment.ColumnSelectorFactory;
+
 import java.util.List;
 
 public interface BooleanFilter extends Filter
@@ -33,16 +35,17 @@ public interface BooleanFilter extends Filter
    * An implementation should either:
    * - return a ValueMatcher that checks row values, using the provided ValueMatcherFactory
    * - or, if possible, get a bitmap index for this filter using the BitmapIndexSelector, and
-   *   return a ValueMatcher that checks the current row offset, created using the bitmap index.
+   * return a ValueMatcher that checks the current row offset, created using the bitmap index.
    *
-   * @param selector Object used to retrieve bitmap indexes
-   * @param valueMatcherFactory Object used to create ValueMatchers
+   * @param selector                Object used to retrieve bitmap indexes
+   * @param columnSelectorFactory   Object used to select columns for making ValueMatchers
    * @param rowOffsetMatcherFactory Object used to create RowOffsetMatchers
+   *
    * @return ValueMatcher that applies this filter
    */
   public ValueMatcher makeMatcher(
       BitmapIndexSelector selector,
-      ValueMatcherFactory valueMatcherFactory,
+      ColumnSelectorFactory columnSelectorFactory,
       RowOffsetMatcherFactory rowOffsetMatcherFactory
   );
 }

processing/src/main/java/io/druid/query/filter/BoundDimFilter.java

@@ -314,22 +314,27 @@ public String toString()
 
   private Supplier<DruidLongPredicate> makeLongPredicateSupplier()
   {
-    return new Supplier<DruidLongPredicate>()
+    class BoundLongPredicateSupplier implements Supplier<DruidLongPredicate>
     {
       private final Object initLock = new Object();
+
+      // longsInitialized is volatile since it establishes the happens-before relationship on
+      // writes/reads to the rest of the fields (it's written last and read first).
       private volatile boolean longsInitialized = false;
-      private volatile boolean matchesAnything = true;
-      private volatile boolean hasLowerLongBoundVolatile;
-      private volatile boolean hasUpperLongBoundVolatile;
-      private volatile long lowerLongBoundVolatile;
-      private volatile long upperLongBoundVolatile;
+
+      // Other fields are not volatile.
+      private boolean matchesNothing;
+      private boolean hasLowerLongBound;
+      private boolean hasUpperLongBound;
+      private long lowerLongBound;
+      private long upperLongBound;
 
       @Override
       public DruidLongPredicate get()
       {
         initLongData();
 
-        if (!matchesAnything) {
+        if (matchesNothing) {
           return new DruidLongPredicate()
           {
             @Override
@@ -342,10 +347,10 @@ public boolean applyLong(long input)
 
         return new DruidLongPredicate()
         {
-          private final boolean hasLowerLongBound = hasLowerLongBoundVolatile;
-          private final boolean hasUpperLongBound = hasUpperLongBoundVolatile;
-          private final long lowerLongBound = hasLowerLongBound ? lowerLongBoundVolatile : 0L;
-          private final long upperLongBound = hasUpperLongBound ? upperLongBoundVolatile : 0L;
+          private final boolean hasLowerLongBound = BoundLongPredicateSupplier.this.hasLowerLongBound;
+          private final boolean hasUpperLongBound = BoundLongPredicateSupplier.this.hasUpperLongBound;
+          private final long lowerLongBound = hasLowerLongBound ? BoundLongPredicateSupplier.this.lowerLongBound : 0L;
+          private final long upperLongBound = hasUpperLongBound ? BoundLongPredicateSupplier.this.upperLongBound : 0L;
 
           @Override
           public boolean applyLong(long input)
@@ -382,35 +387,39 @@ private void initLongData()
             return;
           }
 
+          matchesNothing = false;
+
           if (hasLowerBound()) {
             final Long lowerLong = GuavaUtils.tryParseLong(lower);
             if (lowerLong == null) {
-              matchesAnything = false;
-              return;
+              // Unparseable values fall before all actual numbers, so all numbers will match the lower bound.
+              hasLowerLongBound = false;
+            } else {
+              hasLowerLongBound = true;
+              lowerLongBound = lowerLong;
             }
-
-            hasLowerLongBoundVolatile = true;
-            lowerLongBoundVolatile = lowerLong;
           } else {
-            hasLowerLongBoundVolatile = false;
+            hasLowerLongBound = false;
           }
 
           if (hasUpperBound()) {
             Long upperLong = GuavaUtils.tryParseLong(upper);
             if (upperLong == null) {
-              matchesAnything = false;
+              // Unparseable values fall before all actual numbers, so no numbers can match the upper bound.
+              matchesNothing = true;
               return;
             }
 
-            hasUpperLongBoundVolatile = true;
-            upperLongBoundVolatile = upperLong;
+            hasUpperLongBound = true;
+            upperLongBound = upperLong;
           } else {
-            hasUpperLongBoundVolatile = false;
+            hasUpperLongBound = false;
           }
 
           longsInitialized = true;
         }
       }
-    };
+    }
+    return new BoundLongPredicateSupplier();
   }
 }

processing/src/main/java/io/druid/query/filter/Filter.java

@@ -20,6 +20,7 @@
 package io.druid.query.filter;
 
 import io.druid.collections.bitmap.ImmutableBitmap;
+import io.druid.segment.ColumnSelectorFactory;
 
 /**
  */
@@ -40,7 +41,7 @@ public interface Filter
    * @param factory Object used to create ValueMatchers
    * @return ValueMatcher that applies this filter to row values.
    */
-  public ValueMatcher makeMatcher(ValueMatcherFactory factory);
+  public ValueMatcher makeMatcher(ColumnSelectorFactory factory);
 
 
   /**

processing/src/main/java/io/druid/query/filter/StringValueMatcherColumnSelectorStrategy.java

@@ -21,30 +21,29 @@
 
 import com.google.common.base.Predicate;
 import com.google.common.base.Strings;
-import io.druid.query.dimension.DefaultDimensionSpec;
-import io.druid.segment.ColumnSelectorFactory;
 import io.druid.segment.DimensionSelector;
 import io.druid.segment.data.IndexedInts;
+import io.druid.segment.filter.BooleanValueMatcher;
 
 import java.util.BitSet;
 import java.util.Objects;
 
-public class StringValueMatcherColumnSelectorStrategy implements ValueMatcherColumnSelectorStrategy
+public class StringValueMatcherColumnSelectorStrategy implements ValueMatcherColumnSelectorStrategy<DimensionSelector>
 {
   @Override
-  public ValueMatcher getValueMatcher(String columnName, ColumnSelectorFactory cursor, final String value)
+  public ValueMatcher makeValueMatcher(final DimensionSelector selector, final String value)
   {
     final String valueStr = Strings.emptyToNull(value);
-    final DimensionSelector selector = cursor.makeDimensionSelector(
-        new DefaultDimensionSpec(columnName, columnName)
-    );
 
     // if matching against null, rows with size 0 should also match
     final boolean matchNull = Strings.isNullOrEmpty(valueStr);
 
     final int cardinality = selector.getValueCardinality();
 
-    if (cardinality >= 0) {
+    if (cardinality == 0 || (cardinality == 1 && selector.lookupName(0) == null)) {
+      // All values are null or empty rows (which match nulls anyway). No need to check each row.
+      return new BooleanValueMatcher(matchNull);
+    } else if (cardinality >= 0) {
       // Dictionary-encoded dimension. Compare by id instead of by value to save time.
       final int valueId = selector.lookupId(valueStr);
 
@@ -94,17 +93,19 @@ public boolean matches()
   }
 
   @Override
-  public ValueMatcher getValueMatcher(String columnName, ColumnSelectorFactory cursor, final DruidPredicateFactory predicateFactory)
+  public ValueMatcher makeValueMatcher(
+      final DimensionSelector selector,
+      final DruidPredicateFactory predicateFactory
+  )
   {
-    final DimensionSelector selector = cursor.makeDimensionSelector(
-        new DefaultDimensionSpec(columnName, columnName)
-    );
-
     final Predicate<String> predicate = predicateFactory.makeStringPredicate();
     final int cardinality = selector.getValueCardinality();
     final boolean matchNull = predicate.apply(null);
 
-    if (cardinality >= 0) {
+    if (cardinality == 0 || (cardinality == 1 && selector.lookupName(0) == null)) {
+      // All values are null or empty rows (which match nulls anyway). No need to check each row.
+      return new BooleanValueMatcher(matchNull);
+    } else if (cardinality >= 0) {
       // Dictionary-encoded dimension. Check every value; build a bitset of matching ids.
       final BitSet valueIds = new BitSet(cardinality);
       for (int i = 0; i < cardinality; i++) {