This is an automated email from the ASF dual-hosted git repository.

ccaominh pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git


The following commit(s) were added to refs/heads/master by this push:
     new ed981ef  Add DimFilter.toOptimizedFilter(), ensure that join filter 
pre-analysis operates on optimized filters (#10056)
ed981ef is described below

commit ed981ef88e1005d1b1e5235da4995b18288d6bf9
Author: Jonathan Wei <jon-...@users.noreply.github.com>
AuthorDate: Wed Jul 1 22:26:17 2020 -0700

    Add DimFilter.toOptimizedFilter(), ensure that join filter pre-analysis 
operates on optimized filters (#10056)
    
    * Ensure that join filter pre-analysis operates on optimized filters, add 
DimFilter.toOptimizedFilter
    
    * Remove aggressive equality check that was used for testing
    
    * Use Suppliers.memoize
    
    * Checkstyle
---
 .../apache/druid/query/filter/BloomDimFilter.java  |  2 +-
 .../filter/AbstractOptimizableDimFilter.java}      | 35 ++++-----
 .../apache/druid/query/filter/AndDimFilter.java    |  2 +-
 .../apache/druid/query/filter/BoundDimFilter.java  |  2 +-
 .../query/filter/ColumnComparisonDimFilter.java    |  2 +-
 .../org/apache/druid/query/filter/DimFilter.java   | 12 ++++
 .../druid/query/filter/ExpressionDimFilter.java    |  2 +-
 .../druid/query/filter/ExtractionDimFilter.java    |  2 +-
 .../apache/druid/query/filter/FalseDimFilter.java  |  2 +-
 .../org/apache/druid/query/filter/InDimFilter.java |  2 +-
 .../druid/query/filter/IntervalDimFilter.java      |  2 +-
 .../druid/query/filter/JavaScriptDimFilter.java    |  2 +-
 .../apache/druid/query/filter/LikeDimFilter.java   |  2 +-
 .../apache/druid/query/filter/NotDimFilter.java    |  2 +-
 .../org/apache/druid/query/filter/OrDimFilter.java |  2 +-
 .../apache/druid/query/filter/RegexDimFilter.java  |  2 +-
 .../druid/query/filter/SearchQueryDimFilter.java   |  2 +-
 .../druid/query/filter/SelectorDimFilter.java      |  2 +-
 .../druid/query/filter/SpatialDimFilter.java       |  2 +-
 .../apache/druid/query/filter/TrueDimFilter.java   |  2 +-
 .../query/groupby/GroupByQueryQueryToolChest.java  | 10 +--
 .../org/apache/druid/query/scan/ScanQuery.java     |  5 --
 .../druid/query/scan/ScanQueryQueryToolChest.java  |  5 --
 .../org/apache/druid/query/search/SearchQuery.java |  5 --
 .../query/search/SearchQueryQueryToolChest.java    |  5 --
 .../timeseries/TimeseriesQueryQueryToolChest.java  |  5 --
 .../org/apache/druid/query/topn/TopNQuery.java     |  5 --
 .../druid/query/topn/TopNQueryQueryToolChest.java  | 12 ++--
 .../org/apache/druid/segment/filter/Filters.java   |  2 +-
 .../druid/query/filter/FalseDimFilterTest.java     |  5 +-
 .../druid/query/filter/TrueDimFilterTest.java      |  5 +-
 .../apache/druid/sql/calcite/CalciteQueryTest.java | 82 ++++++++++++++++++++++
 32 files changed, 143 insertions(+), 86 deletions(-)

diff --git 
a/extensions-core/druid-bloom-filter/src/main/java/org/apache/druid/query/filter/BloomDimFilter.java
 
b/extensions-core/druid-bloom-filter/src/main/java/org/apache/druid/query/filter/BloomDimFilter.java
index 6918269..a22fec7 100644
--- 
a/extensions-core/druid-bloom-filter/src/main/java/org/apache/druid/query/filter/BloomDimFilter.java
+++ 
b/extensions-core/druid-bloom-filter/src/main/java/org/apache/druid/query/filter/BloomDimFilter.java
@@ -38,7 +38,7 @@ import java.util.Set;
 
 /**
  */
-public class BloomDimFilter implements DimFilter
+public class BloomDimFilter extends AbstractOptimizableDimFilter implements 
DimFilter
 {
 
   private final String dimension;
diff --git 
a/processing/src/test/java/org/apache/druid/query/filter/TrueDimFilterTest.java 
b/processing/src/main/java/org/apache/druid/query/filter/AbstractOptimizableDimFilter.java
similarity index 53%
copy from 
processing/src/test/java/org/apache/druid/query/filter/TrueDimFilterTest.java
copy to 
processing/src/main/java/org/apache/druid/query/filter/AbstractOptimizableDimFilter.java
index 3c88382..24d32d3 100644
--- 
a/processing/src/test/java/org/apache/druid/query/filter/TrueDimFilterTest.java
+++ 
b/processing/src/main/java/org/apache/druid/query/filter/AbstractOptimizableDimFilter.java
@@ -19,29 +19,24 @@
 
 package org.apache.druid.query.filter;
 
-import com.fasterxml.jackson.databind.ObjectMapper;
-import nl.jqno.equalsverifier.EqualsVerifier;
-import org.apache.druid.jackson.DefaultObjectMapper;
-import org.junit.Assert;
-import org.junit.Test;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.google.common.base.Supplier;
+import com.google.common.base.Suppliers;
 
-import java.io.IOException;
-
-public class TrueDimFilterTest
+/**
+ * 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
 {
-  @Test
-  public void testSerde() throws IOException
-  {
-    final ObjectMapper mapper = new DefaultObjectMapper();
-    final TrueDimFilter original = TrueDimFilter.instance();
-    final byte[] bytes = mapper.writeValueAsBytes(original);
-    final TrueDimFilter fromBytes = (TrueDimFilter) mapper.readValue(bytes, 
DimFilter.class);
-    Assert.assertSame(original, fromBytes);
-  }
+  private final Supplier<Filter> cachedOptimizedFilter = Suppliers.memoize(
+      () -> optimize().toFilter()
+  );
 
-  @Test
-  public void testEquals()
+  @JsonIgnore
+  @Override
+  public Filter toOptimizedFilter()
   {
-    EqualsVerifier.forClass(TrueDimFilter.class).usingGetClass().verify();
+    return cachedOptimizedFilter.get();
   }
 }
diff --git 
a/processing/src/main/java/org/apache/druid/query/filter/AndDimFilter.java 
b/processing/src/main/java/org/apache/druid/query/filter/AndDimFilter.java
index 18df9d6..9e29658 100644
--- a/processing/src/main/java/org/apache/druid/query/filter/AndDimFilter.java
+++ b/processing/src/main/java/org/apache/druid/query/filter/AndDimFilter.java
@@ -37,7 +37,7 @@ import java.util.stream.Collectors;
 
 /**
  */
-public class AndDimFilter implements DimFilter
+public class AndDimFilter extends AbstractOptimizableDimFilter implements 
DimFilter
 {
   private static final Joiner AND_JOINER = Joiner.on(" && ");
 
diff --git 
a/processing/src/main/java/org/apache/druid/query/filter/BoundDimFilter.java 
b/processing/src/main/java/org/apache/druid/query/filter/BoundDimFilter.java
index 98190a7..4b6fc87 100644
--- a/processing/src/main/java/org/apache/druid/query/filter/BoundDimFilter.java
+++ b/processing/src/main/java/org/apache/druid/query/filter/BoundDimFilter.java
@@ -47,7 +47,7 @@ import java.nio.ByteBuffer;
 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
diff --git 
a/processing/src/main/java/org/apache/druid/query/filter/ColumnComparisonDimFilter.java
 
b/processing/src/main/java/org/apache/druid/query/filter/ColumnComparisonDimFilter.java
index dd97ce2..bff805c 100644
--- 
a/processing/src/main/java/org/apache/druid/query/filter/ColumnComparisonDimFilter.java
+++ 
b/processing/src/main/java/org/apache/druid/query/filter/ColumnComparisonDimFilter.java
@@ -35,7 +35,7 @@ import java.util.stream.Collectors;
 
 /**
  */
-public class ColumnComparisonDimFilter implements DimFilter
+public class ColumnComparisonDimFilter extends AbstractOptimizableDimFilter 
implements DimFilter
 {
   private static final Joiner COMMA_JOINER = Joiner.on(", ");
 
diff --git 
a/processing/src/main/java/org/apache/druid/query/filter/DimFilter.java 
b/processing/src/main/java/org/apache/druid/query/filter/DimFilter.java
index 6a38457..6058088 100644
--- a/processing/src/main/java/org/apache/druid/query/filter/DimFilter.java
+++ b/processing/src/main/java/org/apache/druid/query/filter/DimFilter.java
@@ -59,6 +59,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.
    *
diff --git 
a/processing/src/main/java/org/apache/druid/query/filter/ExpressionDimFilter.java
 
b/processing/src/main/java/org/apache/druid/query/filter/ExpressionDimFilter.java
index d902d59..2f65cc6 100644
--- 
a/processing/src/main/java/org/apache/druid/query/filter/ExpressionDimFilter.java
+++ 
b/processing/src/main/java/org/apache/druid/query/filter/ExpressionDimFilter.java
@@ -37,7 +37,7 @@ import javax.annotation.Nullable;
 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;
diff --git 
a/processing/src/main/java/org/apache/druid/query/filter/ExtractionDimFilter.java
 
b/processing/src/main/java/org/apache/druid/query/filter/ExtractionDimFilter.java
index fe84a9e..0bf7063 100644
--- 
a/processing/src/main/java/org/apache/druid/query/filter/ExtractionDimFilter.java
+++ 
b/processing/src/main/java/org/apache/druid/query/filter/ExtractionDimFilter.java
@@ -34,7 +34,7 @@ import java.util.Set;
  * 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;
diff --git 
a/processing/src/main/java/org/apache/druid/query/filter/FalseDimFilter.java 
b/processing/src/main/java/org/apache/druid/query/filter/FalseDimFilter.java
index 4b21e5e..14bfbc0 100644
--- a/processing/src/main/java/org/apache/druid/query/filter/FalseDimFilter.java
+++ b/processing/src/main/java/org/apache/druid/query/filter/FalseDimFilter.java
@@ -29,7 +29,7 @@ import javax.annotation.Nullable;
 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();
diff --git 
a/processing/src/main/java/org/apache/druid/query/filter/InDimFilter.java 
b/processing/src/main/java/org/apache/druid/query/filter/InDimFilter.java
index d5ec558..25688ba 100644
--- a/processing/src/main/java/org/apache/druid/query/filter/InDimFilter.java
+++ b/processing/src/main/java/org/apache/druid/query/filter/InDimFilter.java
@@ -61,7 +61,7 @@ import java.util.Objects;
 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
diff --git 
a/processing/src/main/java/org/apache/druid/query/filter/IntervalDimFilter.java 
b/processing/src/main/java/org/apache/druid/query/filter/IntervalDimFilter.java
index c1e7ec0..bb6d569 100644
--- 
a/processing/src/main/java/org/apache/druid/query/filter/IntervalDimFilter.java
+++ 
b/processing/src/main/java/org/apache/druid/query/filter/IntervalDimFilter.java
@@ -42,7 +42,7 @@ import java.util.List;
 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;
diff --git 
a/processing/src/main/java/org/apache/druid/query/filter/JavaScriptDimFilter.java
 
b/processing/src/main/java/org/apache/druid/query/filter/JavaScriptDimFilter.java
index 9329e86..62a379a 100644
--- 
a/processing/src/main/java/org/apache/druid/query/filter/JavaScriptDimFilter.java
+++ 
b/processing/src/main/java/org/apache/druid/query/filter/JavaScriptDimFilter.java
@@ -43,7 +43,7 @@ import java.nio.ByteBuffer;
 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;
diff --git 
a/processing/src/main/java/org/apache/druid/query/filter/LikeDimFilter.java 
b/processing/src/main/java/org/apache/druid/query/filter/LikeDimFilter.java
index 06a6e2b..a92bd44 100644
--- a/processing/src/main/java/org/apache/druid/query/filter/LikeDimFilter.java
+++ b/processing/src/main/java/org/apache/druid/query/filter/LikeDimFilter.java
@@ -41,7 +41,7 @@ import java.util.Objects;
 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.
diff --git 
a/processing/src/main/java/org/apache/druid/query/filter/NotDimFilter.java 
b/processing/src/main/java/org/apache/druid/query/filter/NotDimFilter.java
index 0cd2969..0b15469 100644
--- a/processing/src/main/java/org/apache/druid/query/filter/NotDimFilter.java
+++ b/processing/src/main/java/org/apache/druid/query/filter/NotDimFilter.java
@@ -32,7 +32,7 @@ import java.util.Set;
 
 /**
  */
-public class NotDimFilter implements DimFilter
+public class NotDimFilter extends AbstractOptimizableDimFilter implements 
DimFilter
 {
 
   private final DimFilter field;
diff --git 
a/processing/src/main/java/org/apache/druid/query/filter/OrDimFilter.java 
b/processing/src/main/java/org/apache/druid/query/filter/OrDimFilter.java
index ca8c105..d758d15 100644
--- a/processing/src/main/java/org/apache/druid/query/filter/OrDimFilter.java
+++ b/processing/src/main/java/org/apache/druid/query/filter/OrDimFilter.java
@@ -38,7 +38,7 @@ import java.util.stream.Collectors;
 
 /**
  */
-public class OrDimFilter implements DimFilter
+public class OrDimFilter extends AbstractOptimizableDimFilter implements 
DimFilter
 {
   private static final Joiner OR_JOINER = Joiner.on(" || ");
 
diff --git 
a/processing/src/main/java/org/apache/druid/query/filter/RegexDimFilter.java 
b/processing/src/main/java/org/apache/druid/query/filter/RegexDimFilter.java
index b696ad9..e78a12d 100644
--- a/processing/src/main/java/org/apache/druid/query/filter/RegexDimFilter.java
+++ b/processing/src/main/java/org/apache/druid/query/filter/RegexDimFilter.java
@@ -38,7 +38,7 @@ import java.util.regex.Pattern;
 
 /**
  */
-public class RegexDimFilter implements DimFilter
+public class RegexDimFilter extends AbstractOptimizableDimFilter implements 
DimFilter
 {
   private final String dimension;
   private final String pattern;
diff --git 
a/processing/src/main/java/org/apache/druid/query/filter/SearchQueryDimFilter.java
 
b/processing/src/main/java/org/apache/druid/query/filter/SearchQueryDimFilter.java
index 757cc79..f89d8d2 100644
--- 
a/processing/src/main/java/org/apache/druid/query/filter/SearchQueryDimFilter.java
+++ 
b/processing/src/main/java/org/apache/druid/query/filter/SearchQueryDimFilter.java
@@ -37,7 +37,7 @@ import java.util.Set;
 
 /**
  */
-public class SearchQueryDimFilter implements DimFilter
+public class SearchQueryDimFilter extends AbstractOptimizableDimFilter 
implements DimFilter
 {
   private final String dimension;
   private final SearchQuerySpec query;
diff --git 
a/processing/src/main/java/org/apache/druid/query/filter/SelectorDimFilter.java 
b/processing/src/main/java/org/apache/druid/query/filter/SelectorDimFilter.java
index d27f413..c717dbe 100644
--- 
a/processing/src/main/java/org/apache/druid/query/filter/SelectorDimFilter.java
+++ 
b/processing/src/main/java/org/apache/druid/query/filter/SelectorDimFilter.java
@@ -41,7 +41,7 @@ import java.util.Set;
 /**
  *
  */
-public class SelectorDimFilter implements DimFilter
+public class SelectorDimFilter extends AbstractOptimizableDimFilter implements 
DimFilter
 {
   private final String dimension;
 
diff --git 
a/processing/src/main/java/org/apache/druid/query/filter/SpatialDimFilter.java 
b/processing/src/main/java/org/apache/druid/query/filter/SpatialDimFilter.java
index c7a1248..6620203 100644
--- 
a/processing/src/main/java/org/apache/druid/query/filter/SpatialDimFilter.java
+++ 
b/processing/src/main/java/org/apache/druid/query/filter/SpatialDimFilter.java
@@ -37,7 +37,7 @@ import java.util.Set;
 
 /**
  */
-public class SpatialDimFilter implements DimFilter
+public class SpatialDimFilter extends AbstractOptimizableDimFilter implements 
DimFilter
 {
   private final String dimension;
   private final Bound bound;
diff --git 
a/processing/src/main/java/org/apache/druid/query/filter/TrueDimFilter.java 
b/processing/src/main/java/org/apache/druid/query/filter/TrueDimFilter.java
index af29710..dda337b 100644
--- a/processing/src/main/java/org/apache/druid/query/filter/TrueDimFilter.java
+++ b/processing/src/main/java/org/apache/druid/query/filter/TrueDimFilter.java
@@ -27,7 +27,7 @@ import org.apache.druid.segment.filter.TrueFilter;
 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();
diff --git 
a/processing/src/main/java/org/apache/druid/query/groupby/GroupByQueryQueryToolChest.java
 
b/processing/src/main/java/org/apache/druid/query/groupby/GroupByQueryQueryToolChest.java
index 741b8f6..83378fb 100644
--- 
a/processing/src/main/java/org/apache/druid/query/groupby/GroupByQueryQueryToolChest.java
+++ 
b/processing/src/main/java/org/apache/druid/query/groupby/GroupByQueryQueryToolChest.java
@@ -472,13 +472,9 @@ public class GroupByQueryQueryToolChest extends 
QueryToolChest<ResultRow, GroupB
           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 class GroupByQueryQueryToolChest extends 
QueryToolChest<ResultRow, GroupB
             }
 
             return runner.run(
-                
queryPlus.withQuery(delegateGroupByQuery.withDimensionSpecs(dimensionSpecs)),
+                
queryPlus.withQuery(groupByQuery.withDimensionSpecs(dimensionSpecs)),
                 responseContext
             );
           }
diff --git 
a/processing/src/main/java/org/apache/druid/query/scan/ScanQuery.java 
b/processing/src/main/java/org/apache/druid/query/scan/ScanQuery.java
index be5d24b..b87a1c4 100644
--- a/processing/src/main/java/org/apache/druid/query/scan/ScanQuery.java
+++ b/processing/src/main/java/org/apache/druid/query/scan/ScanQuery.java
@@ -294,11 +294,6 @@ public class ScanQuery extends BaseQuery<ScanResultValue>
     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)
   {
diff --git 
a/processing/src/main/java/org/apache/druid/query/scan/ScanQueryQueryToolChest.java
 
b/processing/src/main/java/org/apache/druid/query/scan/ScanQueryQueryToolChest.java
index cc6ae06..21550e1 100644
--- 
a/processing/src/main/java/org/apache/druid/query/scan/ScanQueryQueryToolChest.java
+++ 
b/processing/src/main/java/org/apache/druid/query/scan/ScanQueryQueryToolChest.java
@@ -118,11 +118,6 @@ public class ScanQueryQueryToolChest extends 
QueryToolChest<ScanResultValue, Sca
   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);
     };
   }
diff --git 
a/processing/src/main/java/org/apache/druid/query/search/SearchQuery.java 
b/processing/src/main/java/org/apache/druid/query/search/SearchQuery.java
index 4182c5b..c2c2fb9 100644
--- a/processing/src/main/java/org/apache/druid/query/search/SearchQuery.java
+++ b/processing/src/main/java/org/apache/druid/query/search/SearchQuery.java
@@ -109,11 +109,6 @@ public class SearchQuery extends 
BaseQuery<Result<SearchResultValue>>
     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()
   {
diff --git 
a/processing/src/main/java/org/apache/druid/query/search/SearchQueryQueryToolChest.java
 
b/processing/src/main/java/org/apache/druid/query/search/SearchQueryQueryToolChest.java
index 08ee67b..4f05633 100644
--- 
a/processing/src/main/java/org/apache/druid/query/search/SearchQueryQueryToolChest.java
+++ 
b/processing/src/main/java/org/apache/druid/query/search/SearchQueryQueryToolChest.java
@@ -322,11 +322,6 @@ public class SearchQueryQueryToolChest extends 
QueryToolChest<Result<SearchResul
   {
     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
diff --git 
a/processing/src/main/java/org/apache/druid/query/timeseries/TimeseriesQueryQueryToolChest.java
 
b/processing/src/main/java/org/apache/druid/query/timeseries/TimeseriesQueryQueryToolChest.java
index 2ce7e3b..2718282 100644
--- 
a/processing/src/main/java/org/apache/druid/query/timeseries/TimeseriesQueryQueryToolChest.java
+++ 
b/processing/src/main/java/org/apache/druid/query/timeseries/TimeseriesQueryQueryToolChest.java
@@ -377,11 +377,6 @@ public class TimeseriesQueryQueryToolChest extends 
QueryToolChest<Result<Timeser
   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);
     };
   }
diff --git 
a/processing/src/main/java/org/apache/druid/query/topn/TopNQuery.java 
b/processing/src/main/java/org/apache/druid/query/topn/TopNQuery.java
index f5bdec1..8db446b 100644
--- a/processing/src/main/java/org/apache/druid/query/topn/TopNQuery.java
+++ b/processing/src/main/java/org/apache/druid/query/topn/TopNQuery.java
@@ -203,11 +203,6 @@ public class TopNQuery extends 
BaseQuery<Result<TopNResultValue>>
     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()
   {
diff --git 
a/processing/src/main/java/org/apache/druid/query/topn/TopNQueryQueryToolChest.java
 
b/processing/src/main/java/org/apache/druid/query/topn/TopNQueryQueryToolChest.java
index 1850125..2401d97 100644
--- 
a/processing/src/main/java/org/apache/druid/query/topn/TopNQueryQueryToolChest.java
+++ 
b/processing/src/main/java/org/apache/druid/query/topn/TopNQueryQueryToolChest.java
@@ -425,14 +425,10 @@ public class TopNQueryQueryToolChest extends 
QueryToolChest<Result<TopNResultVal
   {
     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 class TopNQueryQueryToolChest extends 
QueryToolChest<Result<TopNResultVal
         );
         return runner.run(delegateQueryPlus, responseContext);
       } else {
-        return runner.run(queryPlus.withQuery(delegateTopNQuery), 
responseContext);
+        return runner.run(queryPlus.withQuery(topNQuery), responseContext);
       }
     };
   }
diff --git 
a/processing/src/main/java/org/apache/druid/segment/filter/Filters.java 
b/processing/src/main/java/org/apache/druid/segment/filter/Filters.java
index 990127f..399adb9 100644
--- a/processing/src/main/java/org/apache/druid/segment/filter/Filters.java
+++ b/processing/src/main/java/org/apache/druid/segment/filter/Filters.java
@@ -86,7 +86,7 @@ public class Filters
   @Nullable
   public static Filter toFilter(@Nullable DimFilter dimFilter)
   {
-    return dimFilter == null ? null : dimFilter.toFilter();
+    return dimFilter == null ? null : dimFilter.toOptimizedFilter();
   }
 
   /**
diff --git 
a/processing/src/test/java/org/apache/druid/query/filter/FalseDimFilterTest.java
 
b/processing/src/test/java/org/apache/druid/query/filter/FalseDimFilterTest.java
index a512bca..2469c13 100644
--- 
a/processing/src/test/java/org/apache/druid/query/filter/FalseDimFilterTest.java
+++ 
b/processing/src/test/java/org/apache/druid/query/filter/FalseDimFilterTest.java
@@ -42,6 +42,9 @@ public class FalseDimFilterTest
   @Test
   public void testEquals()
   {
-    EqualsVerifier.forClass(FalseDimFilter.class).usingGetClass().verify();
+    EqualsVerifier.forClass(FalseDimFilter.class)
+                  .usingGetClass()
+                  .withIgnoredFields("cachedOptimizedFilter")
+                  .verify();
   }
 }
diff --git 
a/processing/src/test/java/org/apache/druid/query/filter/TrueDimFilterTest.java 
b/processing/src/test/java/org/apache/druid/query/filter/TrueDimFilterTest.java
index 3c88382..9d504ae 100644
--- 
a/processing/src/test/java/org/apache/druid/query/filter/TrueDimFilterTest.java
+++ 
b/processing/src/test/java/org/apache/druid/query/filter/TrueDimFilterTest.java
@@ -42,6 +42,9 @@ public class TrueDimFilterTest
   @Test
   public void testEquals()
   {
-    EqualsVerifier.forClass(TrueDimFilter.class).usingGetClass().verify();
+    EqualsVerifier.forClass(TrueDimFilter.class)
+                  .usingGetClass()
+                  .withIgnoredFields("cachedOptimizedFilter")
+                  .verify();
   }
 }
diff --git 
a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java 
b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
index 483cdd2..e51e81d 100644
--- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
+++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
@@ -36,10 +36,12 @@ import org.apache.druid.java.util.common.JodaUtils;
 import org.apache.druid.java.util.common.StringUtils;
 import org.apache.druid.java.util.common.granularity.Granularities;
 import org.apache.druid.java.util.common.granularity.PeriodGranularity;
+import org.apache.druid.java.util.common.guava.Sequence;
 import org.apache.druid.math.expr.ExprMacroTable;
 import org.apache.druid.query.Druids;
 import org.apache.druid.query.GlobalTableDataSource;
 import org.apache.druid.query.LookupDataSource;
+import org.apache.druid.query.Query;
 import org.apache.druid.query.QueryContexts;
 import org.apache.druid.query.QueryDataSource;
 import org.apache.druid.query.QueryException;
@@ -85,6 +87,7 @@ import org.apache.druid.query.filter.NotDimFilter;
 import org.apache.druid.query.filter.RegexDimFilter;
 import org.apache.druid.query.filter.SelectorDimFilter;
 import org.apache.druid.query.groupby.GroupByQuery;
+import org.apache.druid.query.groupby.ResultRow;
 import org.apache.druid.query.groupby.orderby.DefaultLimitSpec;
 import org.apache.druid.query.groupby.orderby.OrderByColumnSpec;
 import org.apache.druid.query.groupby.orderby.OrderByColumnSpec.Direction;
@@ -97,6 +100,8 @@ import org.apache.druid.query.topn.NumericTopNMetricSpec;
 import org.apache.druid.query.topn.TopNQueryBuilder;
 import org.apache.druid.segment.column.ValueType;
 import org.apache.druid.segment.join.JoinType;
+import org.apache.druid.server.QueryLifecycle;
+import org.apache.druid.server.QueryLifecycleFactory;
 import org.apache.druid.sql.calcite.expression.DruidExpression;
 import org.apache.druid.sql.calcite.filtration.Filtration;
 import org.apache.druid.sql.calcite.planner.Calcites;
@@ -12004,6 +12009,83 @@ public class CalciteQueryTest extends 
BaseCalciteQueryTest
   }
 
   @Test
+  @Parameters(source = QueryContextForJoinProvider.class)
+  public void testGroupByJoinAsNativeQueryWithUnoptimizedFilter(Map<String, 
Object> queryContext)
+  {
+    // The query below is the same as the inner groupBy on a join datasource 
from the test
+    // testNestedGroupByOnInlineDataSourceWithFilter, except that the selector 
filter
+    // dim1=def has been rewritten into an unoptimized filter, dim1 IN (def).
+    //
+    // The unoptimized filter will be optimized into dim1=def by the query 
toolchests in their
+    // pre-merge decoration function, when it calls DimFilter.optimize().
+    //
+    // This test's goal is to ensure that the join filter rewrites function 
correctly when there are
+    // unoptimized filters in the join query. The rewrite logic must apply to 
the optimized form of the filters,
+    // as this is what will be passed to HashJoinSegmentAdapter.makeCursors(), 
where the result of the join
+    // filter pre-analysis is used.
+    //
+    // A native query is used because the filter types where we support 
optimization are the AND/OR/NOT and
+    // IN filters. However, when expressed in a SQL query, our SQL planning 
layer is smart enough to already apply
+    // these optimizations in the native query it generates, making it 
impossible to test the unoptimized filter forms
+    // using SQL queries.
+    //
+    // The test method is placed here for convenience as this class provides 
the necessary setup.
+    Query query = GroupByQuery
+        .builder()
+        .setDataSource(
+            join(
+                new QueryDataSource(
+                    newScanQueryBuilder()
+                        .dataSource(CalciteTests.DATASOURCE1)
+                        
.intervals(querySegmentSpec(Intervals.of("2001-01-02T00:00:00.000Z/146140482-04-24T15:36:27.903Z")))
+                        .columns("dim1")
+                        
.resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_COMPACTED_LIST)
+                        .context(queryContext)
+                        .build()
+                ),
+                new QueryDataSource(
+                    newScanQueryBuilder()
+                        .dataSource(CalciteTests.DATASOURCE1)
+                        
.intervals(querySegmentSpec(Intervals.of("2001-01-02T00:00:00.000Z/146140482-04-24T15:36:27.903Z")))
+                        .columns("dim1", "m2")
+                        
.resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_COMPACTED_LIST)
+                        .context(queryContext)
+                        .build()
+                ),
+                "j0.",
+                equalsCondition(
+                    DruidExpression.fromColumn("dim1"),
+                    DruidExpression.fromColumn("j0.dim1")
+                ),
+                JoinType.INNER
+            )
+        )
+        .setGranularity(Granularities.ALL)
+        .setInterval(querySegmentSpec(Filtration.eternity()))
+        .setDimFilter(in("dim1", Collections.singletonList("def"), null))  // 
provide an unoptimized IN filter
+        .setDimensions(
+            dimensions(
+                new DefaultDimensionSpec("v0", "d0")
+            )
+        )
+        .setVirtualColumns(expressionVirtualColumn("v0", "'def'", 
ValueType.STRING))
+        .build();
+
+    QueryLifecycleFactory qlf = 
CalciteTests.createMockQueryLifecycleFactory(walker, conglomerate);
+    QueryLifecycle ql = qlf.factorize();
+    Sequence seq = ql.runSimple(
+        query,
+        CalciteTests.SUPER_USER_AUTH_RESULT,
+        null
+    );
+    List<Object> results = seq.toList();
+    Assert.assertEquals(
+        ImmutableList.of(ResultRow.of("def")),
+        results
+    );
+  }
+
+  @Test
   public void testUsingSubqueryAsFilterOnTwoColumns() throws Exception
   {
     testQuery(


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org

Reply via email to