Add DimFilter.toOptimizedFilter(), ensure that join filter pre-analysis operates on optimized filters

extensions-core/druid-bloom-filter/src/main/java/org/apache/druid/query/filter/BloomDimFilter.java

@@ -38,7 +38,7 @@
 
 /**
  */
-public class BloomDimFilter implements DimFilter
+public class BloomDimFilter extends AbstractOptimizableDimFilter implements DimFilter
 {
 
   private final String dimension;

processing/src/main/java/org/apache/druid/query/filter/AbstractOptimizableDimFilter.java

@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF 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 org.apache.druid.query.filter;
+
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.google.common.base.Supplier;
+import com.google.common.base.Suppliers;
+
+/**
+ * Base class for DimFilters that support optimization. This abstract class provides a default implementation of
+ * toOptimizedFilter that relies on the existing optimize() and toFilter() methods. It uses a memoized supplier.
+ */
+abstract class AbstractOptimizableDimFilter implements DimFilter
+{
+  private final Supplier<Filter> cachedOptimizedFilter = Suppliers.memoize(
+      () -> optimize().toFilter()
+  );
+
+  @JsonIgnore
+  @Override
+  public Filter toOptimizedFilter()
+  {
+    return cachedOptimizedFilter.get();
+  }
+}

processing/src/main/java/org/apache/druid/query/filter/AndDimFilter.java

@@ -37,7 +37,7 @@
 
 /**
  */
-public class AndDimFilter implements DimFilter
+public class AndDimFilter extends AbstractOptimizableDimFilter implements DimFilter
 {
   private static final Joiner AND_JOINER = Joiner.on(" && ");
 

processing/src/main/java/org/apache/druid/query/filter/BoundDimFilter.java

@@ -47,7 +47,7 @@
 import java.util.Objects;
 import java.util.Set;
 
-public class BoundDimFilter implements DimFilter
+public class BoundDimFilter extends AbstractOptimizableDimFilter implements DimFilter
 {
   private final String dimension;
   @Nullable

processing/src/main/java/org/apache/druid/query/filter/ColumnComparisonDimFilter.java

@@ -35,7 +35,7 @@
 
 /**
  */
-public class ColumnComparisonDimFilter implements DimFilter
+public class ColumnComparisonDimFilter extends AbstractOptimizableDimFilter implements DimFilter
 {
   private static final Joiner COMMA_JOINER = Joiner.on(", ");
 

processing/src/main/java/org/apache/druid/query/filter/DimFilter.java

@@ -58,6 +58,18 @@ public interface DimFilter extends Cacheable
    */
   DimFilter optimize();
 
+  /**
+   * @return Return a Filter that implements this DimFilter, after applying optimizations to this DimFilter.
+   * A typical implementation will return the result of `optimize().toFilter()`
+   * See abstract base class {@link AbstractOptimizableDimFilter} for a common implementation shared by
+   * current DimFilters.
+   *
+   * The Filter returned by this method across multiple calls must be the same object: parts of the query stack
+   * compare Filters, and returning the same object allows these checks to avoid deep comparisons.
+   * (see {@link org.apache.druid.segment.join.HashJoinSegmentStorageAdapter#makeCursors for an example}
+   */
+  Filter toOptimizedFilter();
+
   /**
    * Returns a Filter that implements this DimFilter. This does not generally involve optimizing the DimFilter,
    * so it does make sense to optimize first and then call toFilter on the resulting DimFilter.

processing/src/main/java/org/apache/druid/query/filter/ExpressionDimFilter.java

@@ -37,7 +37,7 @@
 import java.util.Objects;
 import java.util.Set;
 
-public class ExpressionDimFilter implements DimFilter
+public class ExpressionDimFilter extends AbstractOptimizableDimFilter implements DimFilter
 {
   private final String expression;
   private final Supplier<Expr> parsed;

processing/src/main/java/org/apache/druid/query/filter/ExtractionDimFilter.java

@@ -34,7 +34,7 @@
  * This class is deprecated, use SelectorDimFilter instead: {@link SelectorDimFilter}
  */
 @Deprecated
-public class ExtractionDimFilter implements DimFilter
+public class ExtractionDimFilter extends AbstractOptimizableDimFilter implements DimFilter
 {
   private final String dimension;
   private final String value;

processing/src/main/java/org/apache/druid/query/filter/FalseDimFilter.java

@@ -29,7 +29,7 @@
 import java.util.Collections;
 import java.util.Set;
 
-public class FalseDimFilter implements DimFilter
+public class FalseDimFilter extends AbstractOptimizableDimFilter implements DimFilter
 {
   private static final FalseDimFilter INSTANCE = new FalseDimFilter();
   private static final byte[] CACHE_KEY = new CacheKeyBuilder(DimFilterUtils.FALSE_CACHE_ID).build();

processing/src/main/java/org/apache/druid/query/filter/InDimFilter.java

@@ -61,7 +61,7 @@
 import java.util.Set;
 import java.util.stream.Collectors;
 
-public class InDimFilter implements DimFilter
+public class InDimFilter extends AbstractOptimizableDimFilter implements DimFilter
 {
   // determined through benchmark that binary search on long[] is faster than HashSet until ~16 elements
   // Hashing threshold is not applied to String for now, String still uses ImmutableSortedSet

processing/src/main/java/org/apache/druid/query/filter/IntervalDimFilter.java

@@ -42,7 +42,7 @@
 import java.util.Objects;
 import java.util.Set;
 
-public class IntervalDimFilter implements DimFilter
+public class IntervalDimFilter extends AbstractOptimizableDimFilter implements DimFilter
 {
   private final List<Interval> intervals;
   private final List<Pair<Long, Long>> intervalLongs;

processing/src/main/java/org/apache/druid/query/filter/JavaScriptDimFilter.java

@@ -43,7 +43,7 @@
 import java.util.Objects;
 import java.util.Set;
 
-public class JavaScriptDimFilter implements DimFilter
+public class JavaScriptDimFilter extends AbstractOptimizableDimFilter implements DimFilter
 {
   private final String dimension;
   private final String function;

processing/src/main/java/org/apache/druid/query/filter/LikeDimFilter.java

@@ -41,7 +41,7 @@
 import java.util.Set;
 import java.util.regex.Pattern;
 
-public class LikeDimFilter implements DimFilter
+public class LikeDimFilter extends AbstractOptimizableDimFilter implements DimFilter
 {
   // Regex matching characters that are definitely okay to include unescaped in a regex.
   // Leads to excessively paranoid escaping, although shouldn't affect runtime beyond compiling the regex.

processing/src/main/java/org/apache/druid/query/filter/NotDimFilter.java

@@ -32,7 +32,7 @@
 
 /**
  */
-public class NotDimFilter implements DimFilter
+public class NotDimFilter extends AbstractOptimizableDimFilter implements DimFilter
 {
 
   private final DimFilter field;

processing/src/main/java/org/apache/druid/query/filter/OrDimFilter.java

@@ -38,7 +38,7 @@
 
 /**
  */
-public class OrDimFilter implements DimFilter
+public class OrDimFilter extends AbstractOptimizableDimFilter implements DimFilter
 {
   private static final Joiner OR_JOINER = Joiner.on(" || ");
 

processing/src/main/java/org/apache/druid/query/filter/RegexDimFilter.java

@@ -38,7 +38,7 @@
 
 /**
  */
-public class RegexDimFilter implements DimFilter
+public class RegexDimFilter extends AbstractOptimizableDimFilter implements DimFilter
 {
   private final String dimension;
   private final String pattern;

processing/src/main/java/org/apache/druid/query/filter/SearchQueryDimFilter.java

@@ -37,7 +37,7 @@
 
 /**
  */
-public class SearchQueryDimFilter implements DimFilter
+public class SearchQueryDimFilter extends AbstractOptimizableDimFilter implements DimFilter
 {
   private final String dimension;
   private final SearchQuerySpec query;

processing/src/main/java/org/apache/druid/query/filter/SelectorDimFilter.java

@@ -41,7 +41,7 @@
 /**
  *
  */
-public class SelectorDimFilter implements DimFilter
+public class SelectorDimFilter extends AbstractOptimizableDimFilter implements DimFilter
 {
   private final String dimension;
 

processing/src/main/java/org/apache/druid/query/filter/SpatialDimFilter.java

@@ -37,7 +37,7 @@
 
 /**
  */
-public class SpatialDimFilter implements DimFilter
+public class SpatialDimFilter extends AbstractOptimizableDimFilter implements DimFilter
 {
   private final String dimension;
   private final Bound bound;

processing/src/main/java/org/apache/druid/query/filter/TrueDimFilter.java

@@ -27,7 +27,7 @@
 import java.util.Collections;
 import java.util.Set;
 
-public class TrueDimFilter implements DimFilter
+public class TrueDimFilter extends AbstractOptimizableDimFilter implements DimFilter
 {
   private static final TrueDimFilter INSTANCE = new TrueDimFilter();
   private static final byte[] CACHE_KEY = new CacheKeyBuilder(DimFilterUtils.TRUE_CACHE_ID).build();

processing/src/main/java/org/apache/druid/query/groupby/GroupByQueryQueryToolChest.java

@@ -472,13 +472,9 @@ public QueryRunner<ResultRow> preMergeQueryDecoration(final QueryRunner<ResultRo
           public Sequence<ResultRow> run(QueryPlus<ResultRow> queryPlus, ResponseContext responseContext)
           {
             GroupByQuery groupByQuery = (GroupByQuery) queryPlus.getQuery();
-            if (groupByQuery.getDimFilter() != null) {
-              groupByQuery = groupByQuery.withDimFilter(groupByQuery.getDimFilter().optimize());
-            }
-            final GroupByQuery delegateGroupByQuery = groupByQuery;
             final List<DimensionSpec> dimensionSpecs = new ArrayList<>();
-            final BitSet optimizedDimensions = extractionsToRewrite(delegateGroupByQuery);
-            final List<DimensionSpec> dimensions = delegateGroupByQuery.getDimensions();
+            final BitSet optimizedDimensions = extractionsToRewrite(groupByQuery);
+            final List<DimensionSpec> dimensions = groupByQuery.getDimensions();
             for (int i = 0; i < dimensions.size(); i++) {
               final DimensionSpec dimensionSpec = dimensions.get(i);
               if (optimizedDimensions.get(i)) {
@@ -491,7 +487,7 @@ public Sequence<ResultRow> run(QueryPlus<ResultRow> queryPlus, ResponseContext r
             }
 
             return runner.run(
-                queryPlus.withQuery(delegateGroupByQuery.withDimensionSpecs(dimensionSpecs)),
+                queryPlus.withQuery(groupByQuery.withDimensionSpecs(dimensionSpecs)),
                 responseContext
             );
           }

processing/src/main/java/org/apache/druid/query/scan/ScanQuery.java

@@ -294,11 +294,6 @@ public Query<ScanResultValue> withOverriddenContext(Map<String, Object> contextO
     return Druids.ScanQueryBuilder.copy(this).context(computeOverriddenContext(getContext(), contextOverrides)).build();
   }
 
-  public ScanQuery withDimFilter(DimFilter dimFilter)
-  {
-    return Druids.ScanQueryBuilder.copy(this).filters(dimFilter).build();
-  }
-
   @Override
   public boolean equals(final Object o)
   {

processing/src/main/java/org/apache/druid/query/scan/ScanQueryQueryToolChest.java

@@ -118,11 +118,6 @@ public TypeReference<ScanResultValue> getResultTypeReference()
   public QueryRunner<ScanResultValue> preMergeQueryDecoration(final QueryRunner<ScanResultValue> runner)
   {
     return (queryPlus, responseContext) -> {
-      ScanQuery scanQuery = (ScanQuery) queryPlus.getQuery();
-      if (scanQuery.getFilter() != null) {
-        scanQuery = scanQuery.withDimFilter(scanQuery.getFilter().optimize());
-        queryPlus = queryPlus.withQuery(scanQuery);
-      }
       return runner.run(queryPlus, responseContext);
     };
   }

processing/src/main/java/org/apache/druid/query/search/SearchQuery.java

@@ -109,11 +109,6 @@ public SearchQuery withOverriddenContext(Map<String, Object> contextOverrides)
     return Druids.SearchQueryBuilder.copy(this).context(newContext).build();
   }
 
-  public SearchQuery withDimFilter(DimFilter dimFilter)
-  {
-    return Druids.SearchQueryBuilder.copy(this).filters(dimFilter).build();
-  }
-
   @JsonProperty("filter")
   public DimFilter getDimensionsFilter()
   {

processing/src/main/java/org/apache/druid/query/search/SearchQueryQueryToolChest.java

@@ -322,11 +322,6 @@ public QueryRunner<Result<SearchResultValue>> preMergeQueryDecoration(final Quer
   {
     return new SearchThresholdAdjustingQueryRunner(
         (queryPlus, responseContext) -> {
-          SearchQuery searchQuery = (SearchQuery) queryPlus.getQuery();
-          if (searchQuery.getDimensionsFilter() != null) {
-            searchQuery = searchQuery.withDimFilter(searchQuery.getDimensionsFilter().optimize());
-            queryPlus = queryPlus.withQuery(searchQuery);
-          }
           return runner.run(queryPlus, responseContext);
         },
         config

processing/src/main/java/org/apache/druid/query/timeseries/TimeseriesQueryQueryToolChest.java

@@ -377,11 +377,6 @@ public Result<TimeseriesResultValue> apply(Object input)
   public QueryRunner<Result<TimeseriesResultValue>> preMergeQueryDecoration(final QueryRunner<Result<TimeseriesResultValue>> runner)
   {
     return (queryPlus, responseContext) -> {
-      TimeseriesQuery timeseriesQuery = (TimeseriesQuery) queryPlus.getQuery();
-      if (timeseriesQuery.getDimensionsFilter() != null) {
-        timeseriesQuery = timeseriesQuery.withDimFilter(timeseriesQuery.getDimensionsFilter().optimize());
-        queryPlus = queryPlus.withQuery(timeseriesQuery);
-      }
       return runner.run(queryPlus, responseContext);
     };
   }

processing/src/main/java/org/apache/druid/query/topn/TopNQuery.java

@@ -203,11 +203,6 @@ public TopNQuery withOverriddenContext(Map<String, Object> contextOverrides)
     return new TopNQueryBuilder(this).context(computeOverriddenContext(getContext(), contextOverrides)).build();
   }
 
-  public TopNQuery withDimFilter(DimFilter dimFilter)
-  {
-    return new TopNQueryBuilder(this).filters(dimFilter).build();
-  }
-
   @Override
   public String toString()
   {

processing/src/main/java/org/apache/druid/query/topn/TopNQueryQueryToolChest.java

@@ -425,14 +425,10 @@ public QueryRunner<Result<TopNResultValue>> preMergeQueryDecoration(final QueryR
   {
     return (queryPlus, responseContext) -> {
       TopNQuery topNQuery = (TopNQuery) queryPlus.getQuery();
-      if (topNQuery.getDimensionsFilter() != null) {
-        topNQuery = topNQuery.withDimFilter(topNQuery.getDimensionsFilter().optimize());
-      }
-      final TopNQuery delegateTopNQuery = topNQuery;
-      if (TopNQueryEngine.canApplyExtractionInPost(delegateTopNQuery)) {
-        final DimensionSpec dimensionSpec = delegateTopNQuery.getDimensionSpec();
+      if (TopNQueryEngine.canApplyExtractionInPost(topNQuery)) {
+        final DimensionSpec dimensionSpec = topNQuery.getDimensionSpec();
         QueryPlus<Result<TopNResultValue>> delegateQueryPlus = queryPlus.withQuery(
-            delegateTopNQuery.withDimensionSpec(
+            topNQuery.withDimensionSpec(
                 new DefaultDimensionSpec(
                     dimensionSpec.getDimension(),
                     dimensionSpec.getOutputName()
@@ -441,7 +437,7 @@ public QueryRunner<Result<TopNResultValue>> preMergeQueryDecoration(final QueryR
         );
         return runner.run(delegateQueryPlus, responseContext);
       } else {
-        return runner.run(queryPlus.withQuery(delegateTopNQuery), responseContext);
+        return runner.run(queryPlus.withQuery(topNQuery), responseContext);
       }
     };
   }

processing/src/main/java/org/apache/druid/segment/filter/Filters.java

@@ -86,7 +86,7 @@ public static Set<Filter> toFilters(List<DimFilter> dimFilters)
   @Nullable
   public static Filter toFilter(@Nullable DimFilter dimFilter)
   {
-    return dimFilter == null ? null : dimFilter.toFilter();
+    return dimFilter == null ? null : dimFilter.toOptimizedFilter();
   }
 
   /**

processing/src/test/java/org/apache/druid/query/filter/FalseDimFilterTest.java

@@ -42,6 +42,9 @@ public void testSerde() throws IOException
   @Test
   public void testEquals()
   {
-    EqualsVerifier.forClass(FalseDimFilter.class).usingGetClass().verify();
+    EqualsVerifier.forClass(FalseDimFilter.class)
+                  .usingGetClass()
+                  .withIgnoredFields("cachedOptimizedFilter")
+                  .verify();
   }
 }

processing/src/test/java/org/apache/druid/query/filter/TrueDimFilterTest.java

@@ -42,6 +42,9 @@ public void testSerde() throws IOException
   @Test
   public void testEquals()
   {
-    EqualsVerifier.forClass(TrueDimFilter.class).usingGetClass().verify();
+    EqualsVerifier.forClass(TrueDimFilter.class)
+                  .usingGetClass()
+                  .withIgnoredFields("cachedOptimizedFilter")
+                  .verify();
   }
 }