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