From 3c84e628b960079049d1ad7aef55a58cb3fc3c0c Mon Sep 17 00:00:00 2001 From: jon-wei Date: Tue, 2 Jun 2020 21:50:34 -0700 Subject: [PATCH 1/9] Fix join filter rewrites with nested queries --- .../benchmark/JoinAndLookupBenchmark.java | 35 +- .../druid/segment/join/HashJoinSegment.java | 12 +- .../join/HashJoinSegmentStorageAdapter.java | 45 +- .../apache/druid/segment/join/Joinables.java | 11 +- .../join/filter/JoinFilterAnalyzer.java | 6 +- .../join/filter/JoinFilterPreAnalysis.java | 44 +- .../rewrite/JoinFilterPreAnalysisGroup.java | 73 + ...BaseHashJoinSegmentStorageAdapterTest.java | 12 +- .../HashJoinSegmentStorageAdapterTest.java | 252 +--- .../segment/join/HashJoinSegmentTest.java | 19 +- .../segment/join/JoinFilterAnalyzerTest.java | 1213 ++++++++--------- .../druid/sql/calcite/CalciteQueryTest.java | 143 +- 12 files changed, 879 insertions(+), 986 deletions(-) create mode 100644 processing/src/main/java/org/apache/druid/segment/join/filter/rewrite/JoinFilterPreAnalysisGroup.java diff --git a/benchmarks/src/test/java/org/apache/druid/benchmark/JoinAndLookupBenchmark.java b/benchmarks/src/test/java/org/apache/druid/benchmark/JoinAndLookupBenchmark.java index cb630a2e400b..4c707380d32a 100644 --- a/benchmarks/src/test/java/org/apache/druid/benchmark/JoinAndLookupBenchmark.java +++ b/benchmarks/src/test/java/org/apache/druid/benchmark/JoinAndLookupBenchmark.java @@ -48,9 +48,7 @@ import org.apache.druid.segment.join.JoinTestHelper; import org.apache.druid.segment.join.JoinType; import org.apache.druid.segment.join.JoinableClause; -import org.apache.druid.segment.join.filter.JoinFilterAnalyzer; -import org.apache.druid.segment.join.filter.JoinFilterPreAnalysis; -import org.apache.druid.segment.join.filter.JoinableClauses; +import org.apache.druid.segment.join.filter.rewrite.JoinFilterPreAnalysisGroup; import org.apache.druid.segment.join.lookup.LookupJoinable; import org.apache.druid.segment.join.table.IndexedTableJoinable; import org.apache.druid.segment.virtual.ExpressionVirtualColumn; @@ -140,19 +138,17 @@ public void setup() throws IOException ) ) ); - JoinFilterPreAnalysis preAnalysisLookupStringKey = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClausesLookupStringKey), - VirtualColumns.EMPTY, - null, + JoinFilterPreAnalysisGroup preAnalysisGroupLookupStringKey = new JoinFilterPreAnalysisGroup( false, false, false, 0 ); + hashJoinLookupStringKeySegment = new HashJoinSegment( baseSegment, joinableClausesLookupStringKey, - preAnalysisLookupStringKey + preAnalysisGroupLookupStringKey ); List joinableClausesLookupLongKey = ImmutableList.of( @@ -167,10 +163,8 @@ public void setup() throws IOException ) ) ); - JoinFilterPreAnalysis preAnalysisLookupLongKey = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClausesLookupLongKey), - VirtualColumns.EMPTY, - null, + + JoinFilterPreAnalysisGroup preAnalysisGroupLookupLongKey = new JoinFilterPreAnalysisGroup( false, false, false, @@ -179,7 +173,7 @@ public void setup() throws IOException hashJoinLookupLongKeySegment = new HashJoinSegment( baseSegment, joinableClausesLookupLongKey, - preAnalysisLookupLongKey + preAnalysisGroupLookupLongKey ); List joinableClausesIndexedTableStringKey = ImmutableList.of( @@ -194,10 +188,8 @@ public void setup() throws IOException ) ) ); - JoinFilterPreAnalysis preAnalysisIndexedTableStringKey = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClausesIndexedTableStringKey), - VirtualColumns.EMPTY, - null, + + JoinFilterPreAnalysisGroup preAnalysisGroupIndexedStringKey = new JoinFilterPreAnalysisGroup( false, false, false, @@ -206,7 +198,7 @@ public void setup() throws IOException hashJoinIndexedTableStringKeySegment = new HashJoinSegment( baseSegment, joinableClausesIndexedTableStringKey, - preAnalysisIndexedTableStringKey + preAnalysisGroupIndexedStringKey ); List joinableClausesIndexedTableLonggKey = ImmutableList.of( @@ -221,10 +213,7 @@ public void setup() throws IOException ) ) ); - JoinFilterPreAnalysis preAnalysisIndexedTableLongKey = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClausesIndexedTableLonggKey), - VirtualColumns.EMPTY, - null, + JoinFilterPreAnalysisGroup preAnalysisGroupIndexedLongKey = new JoinFilterPreAnalysisGroup( false, false, false, @@ -233,7 +222,7 @@ public void setup() throws IOException hashJoinIndexedTableLongKeySegment = new HashJoinSegment( baseSegment, joinableClausesIndexedTableLonggKey, - preAnalysisIndexedTableLongKey + preAnalysisGroupIndexedLongKey ); final Map countryCodeToNameMap = JoinTestHelper.createCountryIsoCodeToNameLookup().getMap(); diff --git a/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegment.java b/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegment.java index d321af958008..8a8c12933d2b 100644 --- a/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegment.java +++ b/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegment.java @@ -24,7 +24,7 @@ import org.apache.druid.segment.QueryableIndex; import org.apache.druid.segment.Segment; import org.apache.druid.segment.StorageAdapter; -import org.apache.druid.segment.join.filter.JoinFilterPreAnalysis; +import org.apache.druid.segment.join.filter.rewrite.JoinFilterPreAnalysisGroup; import org.apache.druid.timeline.SegmentId; import org.joda.time.Interval; @@ -41,23 +41,23 @@ public class HashJoinSegment extends AbstractSegment { private final Segment baseSegment; private final List clauses; - private final JoinFilterPreAnalysis joinFilterPreAnalysis; + private final JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup; /** * @param baseSegment The left-hand side base segment * @param clauses The right-hand side clauses. The caller is responsible for ensuring that there are no * duplicate prefixes or prefixes that shadow each other across the clauses - * @param joinFilterPreAnalysis Pre-analysis computed by {@link org.apache.druid.segment.join.filter.JoinFilterAnalyzer#computeJoinFilterPreAnalysis} + * @param joinFilterPreAnalysisGroup Pre-analysis computed by {@link org.apache.druid.segment.join.filter.JoinFilterAnalyzer#computeJoinFilterPreAnalysis} */ public HashJoinSegment( Segment baseSegment, List clauses, - JoinFilterPreAnalysis joinFilterPreAnalysis + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup ) { this.baseSegment = baseSegment; this.clauses = clauses; - this.joinFilterPreAnalysis = joinFilterPreAnalysis; + this.joinFilterPreAnalysisGroup = joinFilterPreAnalysisGroup; // Verify 'clauses' is nonempty (otherwise it's a waste to create this object, and the caller should know) if (clauses.isEmpty()) { @@ -90,7 +90,7 @@ public QueryableIndex asQueryableIndex() @Override public StorageAdapter asStorageAdapter() { - return new HashJoinSegmentStorageAdapter(baseSegment.asStorageAdapter(), clauses, joinFilterPreAnalysis); + return new HashJoinSegmentStorageAdapter(baseSegment.asStorageAdapter(), clauses, joinFilterPreAnalysisGroup); } @Override diff --git a/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapter.java b/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapter.java index 65c25d498b56..6e3ad7708d4f 100644 --- a/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapter.java +++ b/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapter.java @@ -21,7 +21,6 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Lists; -import org.apache.druid.java.util.common.ISE; import org.apache.druid.java.util.common.granularity.Granularity; import org.apache.druid.java.util.common.guava.Sequence; import org.apache.druid.java.util.common.guava.Sequences; @@ -38,6 +37,8 @@ import org.apache.druid.segment.join.filter.JoinFilterAnalyzer; import org.apache.druid.segment.join.filter.JoinFilterPreAnalysis; import org.apache.druid.segment.join.filter.JoinFilterSplit; +import org.apache.druid.segment.join.filter.JoinableClauses; +import org.apache.druid.segment.join.filter.rewrite.JoinFilterPreAnalysisGroup; import org.joda.time.DateTime; import org.joda.time.Interval; @@ -47,7 +48,6 @@ import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; -import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -55,22 +55,22 @@ public class HashJoinSegmentStorageAdapter implements StorageAdapter { private final StorageAdapter baseAdapter; private final List clauses; - private final JoinFilterPreAnalysis joinFilterPreAnalysis; + private final JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup; /** * @param baseAdapter A StorageAdapter for the left-hand side base segment * @param clauses The right-hand side clauses. The caller is responsible for ensuring that there are no - * duplicate prefixes or prefixes that shadow each other across the clauses + * @param joinFilterPreAnalysisGroup */ HashJoinSegmentStorageAdapter( StorageAdapter baseAdapter, List clauses, - final JoinFilterPreAnalysis joinFilterPreAnalysis + final JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup ) { this.baseAdapter = baseAdapter; this.clauses = clauses; - this.joinFilterPreAnalysis = joinFilterPreAnalysis; + this.joinFilterPreAnalysisGroup = joinFilterPreAnalysisGroup; } @Override @@ -209,13 +209,36 @@ public Sequence makeCursors( @Nullable final QueryMetrics queryMetrics ) { - if (!Objects.equals(joinFilterPreAnalysis.getOriginalFilter(), filter)) { - throw new ISE( - "Filter provided to cursor [%s] does not match join pre-analysis filter [%s]", + JoinFilterPreAnalysis jfpa; + if (filter == null) { + jfpa = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( + joinFilterPreAnalysisGroup, + JoinableClauses.fromList(clauses), + virtualColumns, + null, + joinFilterPreAnalysisGroup.isEnableFilterPushDown(), + joinFilterPreAnalysisGroup.isEnableFilterRewrite(), + joinFilterPreAnalysisGroup.isEnableRewriteValueColumnFilters(), + joinFilterPreAnalysisGroup.getFilterRewriteMaxSize() + ); + } else { + jfpa = joinFilterPreAnalysisGroup.getAnalyses().computeIfAbsent( filter, - joinFilterPreAnalysis.getOriginalFilter() + (theFilter) -> { + return JoinFilterAnalyzer.computeJoinFilterPreAnalysis( + joinFilterPreAnalysisGroup, + JoinableClauses.fromList(clauses), + virtualColumns, + theFilter, + joinFilterPreAnalysisGroup.isEnableFilterPushDown(), + joinFilterPreAnalysisGroup.isEnableFilterRewrite(), + joinFilterPreAnalysisGroup.isEnableRewriteValueColumnFilters(), + joinFilterPreAnalysisGroup.getFilterRewriteMaxSize() + ); + } ); } + final List preJoinVirtualColumns = new ArrayList<>(); final List postJoinVirtualColumns = new ArrayList<>(); @@ -225,7 +248,7 @@ public Sequence makeCursors( postJoinVirtualColumns ); - JoinFilterSplit joinFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); + JoinFilterSplit joinFilterSplit = JoinFilterAnalyzer.splitFilter(jfpa); preJoinVirtualColumns.addAll(joinFilterSplit.getPushDownVirtualColumns()); // Soon, we will need a way to push filters past a join when possible. This could potentially be done right here diff --git a/processing/src/main/java/org/apache/druid/segment/join/Joinables.java b/processing/src/main/java/org/apache/druid/segment/join/Joinables.java index 76bd19e877d2..b5376585d394 100644 --- a/processing/src/main/java/org/apache/druid/segment/join/Joinables.java +++ b/processing/src/main/java/org/apache/druid/segment/join/Joinables.java @@ -25,9 +25,8 @@ import org.apache.druid.segment.Segment; import org.apache.druid.segment.VirtualColumns; import org.apache.druid.segment.column.ColumnHolder; -import org.apache.druid.segment.join.filter.JoinFilterAnalyzer; -import org.apache.druid.segment.join.filter.JoinFilterPreAnalysis; import org.apache.druid.segment.join.filter.JoinableClauses; +import org.apache.druid.segment.join.filter.rewrite.JoinFilterPreAnalysisGroup; import org.apache.druid.utils.JvmUtils; import javax.annotation.Nullable; @@ -108,16 +107,14 @@ public static Function createSegmentMapFn( return Function.identity(); } else { final JoinableClauses joinableClauses = JoinableClauses.createClauses(clauses, joinableFactory); - JoinFilterPreAnalysis jfpa = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - joinableClauses, - virtualColumns, - originalFilter, + final JoinFilterPreAnalysisGroup jfpag = new JoinFilterPreAnalysisGroup( enableFilterPushDown, enableFilterRewrite, enableRewriteValueColumnFilters, filterRewriteMaxSize ); - return baseSegment -> new HashJoinSegment(baseSegment, joinableClauses.getJoinableClauses(), jfpa); + + return baseSegment -> new HashJoinSegment(baseSegment, joinableClauses.getJoinableClauses(), jfpag); } } ); diff --git a/processing/src/main/java/org/apache/druid/segment/join/filter/JoinFilterAnalyzer.java b/processing/src/main/java/org/apache/druid/segment/join/filter/JoinFilterAnalyzer.java index f4f7d31be1cf..bdca7b91c8b1 100644 --- a/processing/src/main/java/org/apache/druid/segment/join/filter/JoinFilterAnalyzer.java +++ b/processing/src/main/java/org/apache/druid/segment/join/filter/JoinFilterAnalyzer.java @@ -32,6 +32,7 @@ import org.apache.druid.segment.filter.Filters; import org.apache.druid.segment.filter.OrFilter; import org.apache.druid.segment.filter.SelectorFilter; +import org.apache.druid.segment.join.filter.rewrite.JoinFilterPreAnalysisGroup; import org.apache.druid.segment.virtual.ExpressionVirtualColumn; import java.util.ArrayList; @@ -95,6 +96,7 @@ public class JoinFilterAnalyzer * @return A JoinFilterPreAnalysis containing information determined in this pre-analysis step. */ public static JoinFilterPreAnalysis computeJoinFilterPreAnalysis( + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup, JoinableClauses joinableClauses, VirtualColumns virtualColumns, Filter originalFilter, @@ -109,9 +111,7 @@ public static JoinFilterPreAnalysis computeJoinFilterPreAnalysis( joinableClauses.splitVirtualColumns(virtualColumns, preJoinVirtualColumns, postJoinVirtualColumns); JoinFilterPreAnalysis.Builder preAnalysisBuilder = - new JoinFilterPreAnalysis.Builder(joinableClauses, originalFilter, postJoinVirtualColumns) - .withEnableFilterPushDown(enableFilterPushDown) - .withEnableFilterRewrite(enableFilterRewrite); + new JoinFilterPreAnalysis.Builder(joinFilterPreAnalysisGroup, joinableClauses, originalFilter, postJoinVirtualColumns); if (originalFilter == null || !enableFilterPushDown) { return preAnalysisBuilder.build(); } diff --git a/processing/src/main/java/org/apache/druid/segment/join/filter/JoinFilterPreAnalysis.java b/processing/src/main/java/org/apache/druid/segment/join/filter/JoinFilterPreAnalysis.java index ae3731db77a2..cd04601786ab 100644 --- a/processing/src/main/java/org/apache/druid/segment/join/filter/JoinFilterPreAnalysis.java +++ b/processing/src/main/java/org/apache/druid/segment/join/filter/JoinFilterPreAnalysis.java @@ -24,6 +24,7 @@ import org.apache.druid.segment.VirtualColumn; import org.apache.druid.segment.join.Equality; import org.apache.druid.segment.join.JoinableClause; +import org.apache.druid.segment.join.filter.rewrite.JoinFilterPreAnalysisGroup; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -51,8 +52,7 @@ public class JoinFilterPreAnalysis private final List normalizedBaseTableClauses; private final List normalizedJoinTableClauses; private final JoinFilterCorrelations correlations; - private final boolean enableFilterPushDown; - private final boolean enableFilterRewrite; + private final JoinFilterPreAnalysisGroup myGroup; private final List postJoinVirtualColumns; private final Equiconditions equiconditions; @@ -63,8 +63,7 @@ private JoinFilterPreAnalysis( final List normalizedBaseTableClauses, final List normalizedJoinTableClauses, JoinFilterCorrelations correlations, - final boolean enableFilterPushDown, - final boolean enableFilterRewrite, + final JoinFilterPreAnalysisGroup myGroup, final Equiconditions equiconditions ) { @@ -74,8 +73,7 @@ private JoinFilterPreAnalysis( this.normalizedBaseTableClauses = normalizedBaseTableClauses; this.normalizedJoinTableClauses = normalizedJoinTableClauses; this.correlations = correlations; - this.enableFilterPushDown = enableFilterPushDown; - this.enableFilterRewrite = enableFilterRewrite; + this.myGroup = myGroup; this.equiconditions = equiconditions; } @@ -116,12 +114,22 @@ public Map> getCorrelationsByD public boolean isEnableFilterPushDown() { - return enableFilterPushDown; + return myGroup.isEnableFilterPushDown(); } public boolean isEnableFilterRewrite() { - return enableFilterRewrite; + return myGroup.isEnableFilterRewrite(); + } + + public boolean isEnableRewriteValueColumnFilters() + { + return myGroup.isEnableRewriteValueColumnFilters(); + } + + public long getFilterRewriteMaxSize() + { + return myGroup.getFilterRewriteMaxSize(); } public Equiconditions getEquiconditions() @@ -134,22 +142,23 @@ public Equiconditions getEquiconditions() */ public static class Builder { + @Nonnull private final JoinFilterPreAnalysisGroup group; @Nonnull private final JoinableClauses joinableClauses; @Nullable private final Filter originalFilter; @Nullable private List normalizedBaseTableClauses; @Nullable private List normalizedJoinTableClauses; @Nullable private JoinFilterCorrelations correlations; - private boolean enableFilterPushDown = false; - private boolean enableFilterRewrite = false; @Nonnull private final List postJoinVirtualColumns; @Nonnull private Equiconditions equiconditions = new Equiconditions(Collections.emptyMap()); public Builder( + @Nonnull JoinFilterPreAnalysisGroup group, @Nonnull JoinableClauses joinableClauses, @Nullable Filter originalFilter, @Nonnull List postJoinVirtualColumns ) { + this.group = group; this.joinableClauses = joinableClauses; this.originalFilter = originalFilter; this.postJoinVirtualColumns = postJoinVirtualColumns; @@ -175,18 +184,6 @@ public Builder withCorrelations( return this; } - public Builder withEnableFilterPushDown(boolean enableFilterPushDown) - { - this.enableFilterPushDown = enableFilterPushDown; - return this; - } - - public Builder withEnableFilterRewrite(boolean enableFilterRewrite) - { - this.enableFilterRewrite = enableFilterRewrite; - return this; - } - public Equiconditions computeEquiconditionsFromJoinableClauses() { Map> equiconditionsMap = new HashMap<>(); @@ -212,8 +209,7 @@ public JoinFilterPreAnalysis build() normalizedBaseTableClauses, normalizedJoinTableClauses, correlations, - enableFilterPushDown, - enableFilterRewrite, + group, equiconditions ); } diff --git a/processing/src/main/java/org/apache/druid/segment/join/filter/rewrite/JoinFilterPreAnalysisGroup.java b/processing/src/main/java/org/apache/druid/segment/join/filter/rewrite/JoinFilterPreAnalysisGroup.java new file mode 100644 index 000000000000..f26f0dc732c3 --- /dev/null +++ b/processing/src/main/java/org/apache/druid/segment/join/filter/rewrite/JoinFilterPreAnalysisGroup.java @@ -0,0 +1,73 @@ +/* + * 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.segment.join.filter.rewrite; + +import org.apache.druid.query.filter.Filter; +import org.apache.druid.segment.join.filter.JoinFilterPreAnalysis; + +import java.util.concurrent.ConcurrentHashMap; + +public class JoinFilterPreAnalysisGroup +{ + private final boolean enableFilterPushDown; + private final boolean enableFilterRewrite; + private final boolean enableRewriteValueColumnFilters; + private final long filterRewriteMaxSize; + private final ConcurrentHashMap analyses; + + public JoinFilterPreAnalysisGroup( + boolean enableFilterPushDown, + boolean enableFilterRewrite, + boolean enableRewriteValueColumnFilters, + long filterRewriteMaxSize + ) + { + this.enableFilterPushDown = enableFilterPushDown; + this.enableFilterRewrite = enableFilterRewrite; + this.enableRewriteValueColumnFilters = enableRewriteValueColumnFilters; + this.filterRewriteMaxSize = filterRewriteMaxSize; + this.analyses = new ConcurrentHashMap<>(); + } + + public boolean isEnableFilterPushDown() + { + return enableFilterPushDown; + } + + public boolean isEnableFilterRewrite() + { + return enableFilterRewrite; + } + + public boolean isEnableRewriteValueColumnFilters() + { + return enableRewriteValueColumnFilters; + } + + public long getFilterRewriteMaxSize() + { + return filterRewriteMaxSize; + } + + public ConcurrentHashMap getAnalyses() + { + return analyses; + } +} diff --git a/processing/src/test/java/org/apache/druid/segment/join/BaseHashJoinSegmentStorageAdapterTest.java b/processing/src/test/java/org/apache/druid/segment/join/BaseHashJoinSegmentStorageAdapterTest.java index 45befeba36ed..16e46ec22bb5 100644 --- a/processing/src/test/java/org/apache/druid/segment/join/BaseHashJoinSegmentStorageAdapterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/join/BaseHashJoinSegmentStorageAdapterTest.java @@ -26,10 +26,7 @@ import org.apache.druid.query.QueryContexts; import org.apache.druid.query.lookup.LookupExtractor; import org.apache.druid.segment.QueryableIndexSegment; -import org.apache.druid.segment.VirtualColumns; -import org.apache.druid.segment.join.filter.JoinFilterAnalyzer; -import org.apache.druid.segment.join.filter.JoinFilterPreAnalysis; -import org.apache.druid.segment.join.filter.JoinableClauses; +import org.apache.druid.segment.join.filter.rewrite.JoinFilterPreAnalysisGroup; import org.apache.druid.segment.join.lookup.LookupJoinable; import org.apache.druid.segment.join.table.IndexedTable; import org.apache.druid.segment.join.table.IndexedTableJoinable; @@ -187,10 +184,7 @@ protected JoinableClause regionToCountry(final JoinType joinType) protected HashJoinSegmentStorageAdapter makeFactToCountrySegment() { - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(ImmutableList.of(factToCountryOnIsoCode(JoinType.LEFT))), - VirtualColumns.EMPTY, - null, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -200,7 +194,7 @@ protected HashJoinSegmentStorageAdapter makeFactToCountrySegment() return new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), ImmutableList.of(factToCountryOnIsoCode(JoinType.LEFT)), - preAnalysis + joinFilterPreAnalysisGroup ); } diff --git a/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapterTest.java b/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapterTest.java index 2e863b2487a2..3e77c5740d7c 100644 --- a/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapterTest.java @@ -23,7 +23,6 @@ import com.google.common.collect.Lists; import org.apache.druid.common.config.NullHandling; import org.apache.druid.java.util.common.DateTimes; -import org.apache.druid.java.util.common.ISE; import org.apache.druid.java.util.common.Intervals; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.java.util.common.granularity.Granularities; @@ -37,9 +36,7 @@ import org.apache.druid.segment.column.ColumnCapabilities; import org.apache.druid.segment.column.ValueType; import org.apache.druid.segment.filter.SelectorFilter; -import org.apache.druid.segment.join.filter.JoinFilterAnalyzer; -import org.apache.druid.segment.join.filter.JoinFilterPreAnalysis; -import org.apache.druid.segment.join.filter.JoinableClauses; +import org.apache.druid.segment.join.filter.rewrite.JoinFilterPreAnalysisGroup; import org.apache.druid.segment.join.lookup.LookupJoinable; import org.apache.druid.segment.join.table.IndexedTableJoinable; import org.apache.druid.segment.virtual.ExpressionVirtualColumn; @@ -302,10 +299,7 @@ public void test_makeCursors_factToCountryLeft() { List joinableClauses = ImmutableList.of(factToCountryOnIsoCode(JoinType.LEFT)); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - null, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -316,7 +310,7 @@ public void test_makeCursors_factToCountryLeft() new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( null, Intervals.ETERNITY, @@ -371,10 +365,7 @@ public void test_makeCursors_factToCountryLeftUsingLookup() { List joinableClauses = ImmutableList.of(factToCountryNameUsingIsoCodeLookup(JoinType.LEFT)); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - null, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -385,7 +376,7 @@ public void test_makeCursors_factToCountryLeftUsingLookup() new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( null, Intervals.ETERNITY, @@ -438,10 +429,7 @@ public void test_makeCursors_factToCountryLeftUsingLookup() public void test_makeCursors_factToCountryInner() { List joinableClauses = ImmutableList.of(factToCountryOnIsoCode(JoinType.INNER)); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - null, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -452,7 +440,7 @@ public void test_makeCursors_factToCountryInner() new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( null, Intervals.ETERNITY, @@ -500,10 +488,7 @@ public void test_makeCursors_factToCountryInner() public void test_makeCursors_factToCountryInnerUsingLookup() { List joinableClauses = ImmutableList.of(factToCountryNameUsingIsoCodeLookup(JoinType.INNER)); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - null, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -514,7 +499,7 @@ public void test_makeCursors_factToCountryInnerUsingLookup() new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( null, Intervals.ETERNITY, @@ -564,10 +549,7 @@ public void test_makeCursors_factToCountryInnerUsingCountryNumber() // is interpreted as 0 (a.k.a. Australia). List joinableClauses = ImmutableList.of(factToCountryOnNumber(JoinType.INNER)); Filter filter = new SelectorDimFilter("channel", "#en.wikipedia", null).toFilter(); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - filter, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -578,7 +560,7 @@ public void test_makeCursors_factToCountryInnerUsingCountryNumber() new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( filter, Intervals.ETERNITY, @@ -634,10 +616,7 @@ public void test_makeCursors_factToCountryInnerUsingCountryNumberUsingLookup() // is interpreted as 0 (a.k.a. Australia). List joinableClauses = ImmutableList.of(factToCountryNameUsingNumberLookup(JoinType.INNER)); Filter filter = new SelectorDimFilter("channel", "#en.wikipedia", null).toFilter(); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - filter, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -648,7 +627,7 @@ public void test_makeCursors_factToCountryInnerUsingCountryNumberUsingLookup() new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( filter, Intervals.ETERNITY, @@ -700,10 +679,7 @@ public void test_makeCursors_factToCountryLeftWithFilterOnFacts() { List joinableClauses = ImmutableList.of(factToCountryOnIsoCode(JoinType.LEFT)); Filter filter = new SelectorDimFilter("channel", "#de.wikipedia", null).toFilter(); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - filter, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -714,7 +690,7 @@ public void test_makeCursors_factToCountryLeftWithFilterOnFacts() new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( filter, Intervals.ETERNITY, @@ -741,10 +717,7 @@ public void test_makeCursors_factToCountryLeftWithFilterOnFactsUsingLookup() { List joinableClauses = ImmutableList.of(factToCountryNameUsingIsoCodeLookup(JoinType.LEFT)); Filter filter = new SelectorDimFilter("channel", "#de.wikipedia", null).toFilter(); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - filter, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -755,7 +728,7 @@ public void test_makeCursors_factToCountryLeftWithFilterOnFactsUsingLookup() new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( filter, Intervals.ETERNITY, @@ -781,10 +754,7 @@ public void test_makeCursors_factToCountryRightWithFilterOnLeftIsNull() { List joinableClauses = ImmutableList.of(factToCountryOnIsoCode(JoinType.RIGHT)); Filter filter = new SelectorDimFilter("channel", null, null).toFilter(); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - filter, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -795,7 +765,7 @@ public void test_makeCursors_factToCountryRightWithFilterOnLeftIsNull() new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( filter, Intervals.ETERNITY, @@ -824,10 +794,7 @@ public void test_makeCursors_factToCountryRightWithFilterOnLeftIsNullUsingLookup { List joinableClauses = ImmutableList.of(factToCountryNameUsingIsoCodeLookup(JoinType.RIGHT)); Filter filter = new SelectorDimFilter("channel", null, null).toFilter(); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - filter, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -838,7 +805,7 @@ public void test_makeCursors_factToCountryRightWithFilterOnLeftIsNullUsingLookup new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( filter, Intervals.ETERNITY, @@ -866,10 +833,7 @@ public void test_makeCursors_factToCountryFullWithFilterOnLeftIsNull() { List joinableClauses = ImmutableList.of(factToCountryOnIsoCode(JoinType.FULL)); Filter filter = new SelectorDimFilter("channel", null, null).toFilter(); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - filter, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -880,7 +844,7 @@ public void test_makeCursors_factToCountryFullWithFilterOnLeftIsNull() new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( filter, Intervals.ETERNITY, @@ -909,10 +873,7 @@ public void test_makeCursors_factToCountryFullWithFilterOnLeftIsNullUsingLookup( { List joinableClauses = ImmutableList.of(factToCountryNameUsingIsoCodeLookup(JoinType.FULL)); Filter filter = new SelectorDimFilter("channel", null, null).toFilter(); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - filter, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -923,7 +884,7 @@ public void test_makeCursors_factToCountryFullWithFilterOnLeftIsNullUsingLookup( new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( filter, Intervals.ETERNITY, @@ -956,10 +917,7 @@ public void test_makeCursors_factToCountryRightWithFilterOnJoinable() null ).toFilter(); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - filter, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -970,7 +928,7 @@ public void test_makeCursors_factToCountryRightWithFilterOnJoinable() new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( filter, Intervals.ETERNITY, @@ -1003,10 +961,7 @@ public void test_makeCursors_factToCountryRightWithFilterOnJoinableUsingLookup() null ).toFilter(); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - filter, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -1017,7 +972,7 @@ public void test_makeCursors_factToCountryRightWithFilterOnJoinableUsingLookup() new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( filter, Intervals.ETERNITY, @@ -1050,10 +1005,7 @@ public void test_makeCursors_factToCountryLeftWithFilterOnJoinable() new SelectorDimFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "countryNumber", "10", null) ).toFilter(); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - filter, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -1064,7 +1016,7 @@ public void test_makeCursors_factToCountryLeftWithFilterOnJoinable() new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( filter, Intervals.ETERNITY, @@ -1097,10 +1049,7 @@ public void test_makeCursors_factToCountryLeftWithFilterOnJoinableUsingLookup() new SelectorDimFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "v", "Norway", null) ).toFilter(); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - filter, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -1111,7 +1060,7 @@ public void test_makeCursors_factToCountryLeftWithFilterOnJoinableUsingLookup() new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( filter, Intervals.ETERNITY, @@ -1157,10 +1106,7 @@ public void test_makeCursors_factToCountryInnerWithFilterInsteadOfRealJoinCondit ExprMacroTable.nil() ).toFilter(); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - filter, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -1171,7 +1117,7 @@ public void test_makeCursors_factToCountryInnerWithFilterInsteadOfRealJoinCondit new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( filter, Intervals.ETERNITY, @@ -1239,10 +1185,7 @@ public void test_makeCursors_factToCountryInnerWithFilterInsteadOfRealJoinCondit ExprMacroTable.nil() ).toFilter(); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - filter, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -1253,7 +1196,7 @@ public void test_makeCursors_factToCountryInnerWithFilterInsteadOfRealJoinCondit new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( filter, Intervals.ETERNITY, @@ -1304,10 +1247,7 @@ public void test_makeCursors_factToRegionToCountryLeft() regionToCountry(JoinType.LEFT) ); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - null, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -1318,7 +1258,7 @@ public void test_makeCursors_factToRegionToCountryLeft() new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( null, Intervals.ETERNITY, @@ -1383,10 +1323,7 @@ public void test_makeCursors_factToCountryAlwaysTrue() ); Filter filter = new SelectorDimFilter("channel", "#de.wikipedia", null).toFilter(); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - filter, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -1396,7 +1333,7 @@ public void test_makeCursors_factToCountryAlwaysTrue() new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( filter, Intervals.ETERNITY, @@ -1450,10 +1387,7 @@ public void test_makeCursors_factToCountryAlwaysFalse() Filter filter = new SelectorDimFilter("channel", "#de.wikipedia", null).toFilter(); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - filter, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -1464,7 +1398,7 @@ public void test_makeCursors_factToCountryAlwaysFalse() new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( filter, Intervals.ETERNITY, @@ -1501,10 +1435,7 @@ public void test_makeCursors_factToCountryAlwaysTrueUsingLookup() Filter filter = new SelectorDimFilter("channel", "#de.wikipedia", null).toFilter(); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - filter, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -1515,7 +1446,7 @@ public void test_makeCursors_factToCountryAlwaysTrueUsingLookup() new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( filter, Intervals.ETERNITY, @@ -1569,10 +1500,7 @@ public void test_makeCursors_factToCountryAlwaysFalseUsingLookup() Filter filter = new SelectorDimFilter("channel", "#de.wikipedia", null).toFilter(); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - filter, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -1583,7 +1511,7 @@ public void test_makeCursors_factToCountryAlwaysFalseUsingLookup() new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( filter, Intervals.ETERNITY, @@ -1629,10 +1557,7 @@ public void test_makeCursors_factToCountryUsingVirtualColumn() ) ); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - virtualColumns, - null, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -1643,7 +1568,7 @@ public void test_makeCursors_factToCountryUsingVirtualColumn() new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( null, Intervals.ETERNITY, @@ -1695,10 +1620,7 @@ public void test_makeCursors_factToCountryUsingVirtualColumnUsingLookup() ) ); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - virtualColumns, - null, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -1709,7 +1631,7 @@ public void test_makeCursors_factToCountryUsingVirtualColumnUsingLookup() new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( null, Intervals.ETERNITY, @@ -1753,10 +1675,7 @@ public void test_makeCursors_factToCountryUsingExpression() ) ); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - null, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -1767,7 +1686,7 @@ public void test_makeCursors_factToCountryUsingExpression() new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( null, Intervals.ETERNITY, @@ -1810,10 +1729,7 @@ public void test_makeCursors_factToCountryUsingExpressionUsingLookup() ) ); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - null, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -1824,7 +1740,7 @@ public void test_makeCursors_factToCountryUsingExpressionUsingLookup() new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( null, Intervals.ETERNITY, @@ -1870,10 +1786,7 @@ public void test_makeCursors_factToRegionTheWrongWay() Filter filter = new SelectorDimFilter("regionIsoCode", "VA", null).toFilter(); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - filter, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -1884,7 +1797,7 @@ public void test_makeCursors_factToRegionTheWrongWay() new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( filter, Intervals.ETERNITY, @@ -1930,10 +1843,7 @@ public void test_makeCursors_errorOnNonEquiJoin() ) ); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - null, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -1944,7 +1854,7 @@ public void test_makeCursors_errorOnNonEquiJoin() new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( null, Intervals.ETERNITY, @@ -1976,10 +1886,7 @@ public void test_makeCursors_errorOnNonEquiJoinUsingLookup() ) ); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - null, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -1990,7 +1897,7 @@ public void test_makeCursors_errorOnNonEquiJoinUsingLookup() new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( null, Intervals.ETERNITY, @@ -2022,10 +1929,7 @@ public void test_makeCursors_errorOnNonKeyBasedJoin() ) ); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - null, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -2036,7 +1940,7 @@ public void test_makeCursors_errorOnNonKeyBasedJoin() new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( null, Intervals.ETERNITY, @@ -2067,10 +1971,7 @@ public void test_makeCursors_errorOnNonKeyBasedJoinUsingLookup() ) ); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - null, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -2081,7 +1982,7 @@ public void test_makeCursors_errorOnNonKeyBasedJoinUsingLookup() new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( null, Intervals.ETERNITY, @@ -2100,10 +2001,7 @@ public void test_makeCursors_factToCountryLeft_filterExcludesAllLeftRows() Filter originalFilter = new SelectorFilter("page", "this matches nothing"); List joinableClauses = ImmutableList.of(factToCountryOnIsoCode(JoinType.LEFT)); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - originalFilter, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -2114,7 +2012,7 @@ public void test_makeCursors_factToCountryLeft_filterExcludesAllLeftRows() new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( originalFilter, Intervals.ETERNITY, @@ -2139,10 +2037,7 @@ public void test_makeCursors_factToCountryLeft_filterExcludesAllLeftRowsUsingLoo { Filter originalFilter = new SelectorFilter("page", "this matches nothing"); List joinableClauses = ImmutableList.of(factToCountryNameUsingIsoCodeLookup(JoinType.LEFT)); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - originalFilter, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -2153,7 +2048,7 @@ public void test_makeCursors_factToCountryLeft_filterExcludesAllLeftRowsUsingLoo new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( originalFilter, Intervals.ETERNITY, @@ -2172,15 +2067,13 @@ public void test_makeCursors_factToCountryLeft_filterExcludesAllLeftRowsUsingLoo ); } + /* @Test public void test_makeCursors_originalFilterDoesNotMatchPreAnalysis_shouldThrowISE() { List joinableClauses = ImmutableList.of(factToCountryOnIsoCode(JoinType.LEFT)); - JoinFilterPreAnalysis preAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - null, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -2192,7 +2085,7 @@ public void test_makeCursors_originalFilterDoesNotMatchPreAnalysis_shouldThrowIS new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - preAnalysis + joinFilterPreAnalysisGroup ).makeCursors( filter, Intervals.ETERNITY, @@ -2207,4 +2100,5 @@ public void test_makeCursors_originalFilterDoesNotMatchPreAnalysis_shouldThrowIS Assert.assertTrue(e.getMessage().startsWith("Filter provided to cursor [")); } } + */ } diff --git a/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentTest.java b/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentTest.java index 3ff19e4d5918..458828670126 100644 --- a/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentTest.java +++ b/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentTest.java @@ -24,10 +24,7 @@ import org.apache.druid.math.expr.ExprMacroTable; import org.apache.druid.query.QueryContexts; import org.apache.druid.segment.QueryableIndexSegment; -import org.apache.druid.segment.VirtualColumns; -import org.apache.druid.segment.join.filter.JoinFilterAnalyzer; -import org.apache.druid.segment.join.filter.JoinFilterPreAnalysis; -import org.apache.druid.segment.join.filter.JoinableClauses; +import org.apache.druid.segment.join.filter.rewrite.JoinFilterPreAnalysisGroup; import org.apache.druid.segment.join.table.IndexedTableJoinable; import org.apache.druid.timeline.SegmentId; import org.hamcrest.CoreMatchers; @@ -82,10 +79,7 @@ public void setUp() throws IOException ) ); - JoinFilterPreAnalysis joinFilterPreAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - null, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -95,7 +89,7 @@ public void setUp() throws IOException hashJoinSegment = new HashJoinSegment( baseSegment, joinableClauses, - joinFilterPreAnalysis + joinFilterPreAnalysisGroup ); } @@ -107,10 +101,7 @@ public void test_constructor_noClauses() List joinableClauses = ImmutableList.of(); - JoinFilterPreAnalysis joinFilterPreAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - null, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, true, true, @@ -120,7 +111,7 @@ public void test_constructor_noClauses() final HashJoinSegment ignored = new HashJoinSegment( baseSegment, joinableClauses, - joinFilterPreAnalysis + joinFilterPreAnalysisGroup ); } diff --git a/processing/src/test/java/org/apache/druid/segment/join/JoinFilterAnalyzerTest.java b/processing/src/test/java/org/apache/druid/segment/join/JoinFilterAnalyzerTest.java index 965dd2302f11..ef3411d4aacf 100644 --- a/processing/src/test/java/org/apache/druid/segment/join/JoinFilterAnalyzerTest.java +++ b/processing/src/test/java/org/apache/druid/segment/join/JoinFilterAnalyzerTest.java @@ -46,6 +46,7 @@ import org.apache.druid.segment.join.filter.JoinFilterPreAnalysis; import org.apache.druid.segment.join.filter.JoinFilterSplit; import org.apache.druid.segment.join.filter.JoinableClauses; +import org.apache.druid.segment.join.filter.rewrite.JoinFilterPreAnalysisGroup; import org.apache.druid.segment.join.lookup.LookupJoinable; import org.apache.druid.segment.join.table.IndexedTableJoinable; import org.apache.druid.segment.virtual.ExpressionVirtualColumn; @@ -66,24 +67,13 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterOnChannel() regionToCountry(JoinType.LEFT) ); - JoinFilterPreAnalysis joinFilterPreAnalysis = simplePreAnalysis( - joinableClauses, - originalFilter - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - joinFilterPreAnalysis - ); - - JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( - new SelectorFilter("channel", "#en.wikipedia"), - null, - ImmutableSet.of() + joinFilterPreAnalysisGroup ); - JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); - Assert.assertEquals(expectedFilterSplit, actualFilterSplit); JoinTestHelper.verifyCursors( adapter.makeCursors( @@ -116,6 +106,15 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterOnChannel() new Object[]{"History of Fourems", "Fourems Province", "Fourems"} ) ); + + JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( + new SelectorFilter("channel", "#en.wikipedia"), + null, + ImmutableSet.of() + ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); + Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @Test @@ -142,24 +141,13 @@ public void test_filterPushDown_factToRegionExprToCountryLeftFilterOnCountryName regionExprToCountry ); - JoinFilterPreAnalysis joinFilterPreAnalysis = simplePreAnalysis( - joinableClauses, - originalFilter - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - joinFilterPreAnalysis - ); - - JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( - null, - new SelectorFilter("rtc.countryName", "United States"), - ImmutableSet.of() + joinFilterPreAnalysisGroup ); - JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); - Assert.assertEquals(expectedFilterSplit, actualFilterSplit); JoinTestHelper.verifyCursors( adapter.makeCursors( @@ -179,6 +167,15 @@ public void test_filterPushDown_factToRegionExprToCountryLeftFilterOnCountryName new Object[]{"Cream Soda", "Ainigriv", "United States"} ) ); + + JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( + null, + new SelectorFilter("rtc.countryName", "United States"), + ImmutableSet.of() + ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); + Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @Test @@ -196,29 +193,13 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterOnChannelAndCount regionToCountry(JoinType.LEFT) ); - JoinFilterPreAnalysis joinFilterPreAnalysis = simplePreAnalysis( - joinableClauses, - originalFilter - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - joinFilterPreAnalysis - ); - - JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( - new AndFilter( - ImmutableList.of( - new SelectorFilter("channel", "#en.wikipedia"), - new InDimFilter("countryIsoCode", ImmutableSet.of("US"), null, null).toFilter() - ) - ), - new SelectorFilter("rtc.countryName", "United States"), - ImmutableSet.of() + joinFilterPreAnalysisGroup ); - JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); - Assert.assertEquals(expectedFilterSplit, actualFilterSplit); JoinTestHelper.verifyCursors( adapter.makeCursors( @@ -242,6 +223,20 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterOnChannelAndCount new Object[]{"Old Anatolian Turkish", "Virginia", "United States"} ) ); + + JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( + new AndFilter( + ImmutableList.of( + new SelectorFilter("channel", "#en.wikipedia"), + new InDimFilter("countryIsoCode", ImmutableSet.of("US"), null, null).toFilter() + ) + ), + new SelectorFilter("rtc.countryName", "United States"), + ImmutableSet.of() + ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); + Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @Test @@ -261,33 +256,14 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterOnNullColumns() regionToCountry(JoinType.LEFT) ); - JoinFilterPreAnalysis joinFilterPreAnalysis = simplePreAnalysis( - joinableClauses, - originalFilter - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - joinFilterPreAnalysis - ); - - JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( - null, - new AndFilter( - ImmutableList.of( - new SelectorFilter("countryIsoCode", null), - new SelectorFilter("countryNumber", null), - new SelectorFilter("rtc.countryName", null), - new SelectorFilter("r1.regionName", null) - ) - ), - ImmutableSet.of() + joinFilterPreAnalysisGroup ); - JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); - Assert.assertEquals(expectedFilterSplit, actualFilterSplit); - JoinTestHelper.verifyCursors( adapter.makeCursors( originalFilter, @@ -312,8 +288,24 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterOnNullColumns() ) : ImmutableList.of() // when not running in SQL compatible mode, countryNumber does not have nulls ); - } + JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( + null, + new AndFilter( + ImmutableList.of( + new SelectorFilter("countryIsoCode", null), + new SelectorFilter("countryNumber", null), + new SelectorFilter("rtc.countryName", null), + new SelectorFilter("r1.regionName", null) + ) + ), + ImmutableSet.of() + ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); + Assert.assertEquals(expectedFilterSplit, actualFilterSplit); + } + @Test public void test_filterPushDown_factToRegionToCountryLeftFilterOnInvalidColumns() { @@ -331,30 +323,14 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterOnInvalidColumns( ) ); - JoinFilterPreAnalysis joinFilterPreAnalysis = simplePreAnalysis( - joinableClauses, - originalFilter - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - joinFilterPreAnalysis - ); - - JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( - new SelectorFilter("baseTableInvalidColumn", "abcd"), - new AndFilter( - ImmutableList.of( - new SelectorFilter("baseTableInvalidColumn2", null), - new SelectorFilter("rtc.invalidColumn", "abcd"), - new SelectorFilter("r1.invalidColumn", "abcd") - ) - ), - ImmutableSet.of() + joinFilterPreAnalysisGroup ); - JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); - Assert.assertEquals(expectedFilterSplit, actualFilterSplit); JoinTestHelper.verifyCursors( adapter.makeCursors( @@ -372,6 +348,21 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterOnInvalidColumns( ), ImmutableList.of() ); + + JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( + new SelectorFilter("baseTableInvalidColumn", "abcd"), + new AndFilter( + ImmutableList.of( + new SelectorFilter("baseTableInvalidColumn2", null), + new SelectorFilter("rtc.invalidColumn", "abcd"), + new SelectorFilter("r1.invalidColumn", "abcd") + ) + ), + ImmutableSet.of() + ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); + Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @Test @@ -388,15 +379,12 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterOnChannelVirtualC ) ); - JoinFilterPreAnalysis joinFilterPreAnalysis = simplePreAnalysis( - joinableClauses, - originalFilter - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - joinFilterPreAnalysis + joinFilterPreAnalysisGroup ); VirtualColumns virtualColumns = VirtualColumns.create( @@ -410,14 +398,6 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterOnChannelVirtualC ) ); - JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( - new SelectorFilter("v1", "virtual-column-#en.wikipedia"), - null, - ImmutableSet.of() - ); - JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); - Assert.assertEquals(expectedFilterSplit, actualFilterSplit); - JoinTestHelper.verifyCursors( adapter.makeCursors( originalFilter, @@ -449,6 +429,15 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterOnChannelVirtualC new Object[]{"History of Fourems", "Fourems Province", "Fourems"} ) ); + + JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( + new SelectorFilter("v1", "virtual-column-#en.wikipedia"), + null, + ImmutableSet.of() + ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); + Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @Test @@ -472,29 +461,13 @@ public void test_filterPushDown_factToRegionFilterOnRHSRegionNameExprVirtualColu factToRegion(JoinType.LEFT) )); - JoinFilterPreAnalysis joinFilterPreAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - joinableClauses, - virtualColumns, - originalFilter, - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses.getJoinableClauses(), - joinFilterPreAnalysis - ); - - JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( - null, - new SelectorFilter("v0", "VIRGINIA"), - ImmutableSet.of() + joinFilterPreAnalysisGroup ); - JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); - Assert.assertEquals(expectedFilterSplit, actualFilterSplit); JoinTestHelper.verifyCursors( adapter.makeCursors( @@ -513,8 +486,16 @@ public void test_filterPushDown_factToRegionFilterOnRHSRegionNameExprVirtualColu new Object[]{"Old Anatolian Turkish", "VIRGINIA"} ) ); - } + JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( + null, + new SelectorFilter("v0", "VIRGINIA"), + ImmutableSet.of() + ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); + Assert.assertEquals(expectedFilterSplit, actualFilterSplit); + } @Test public void test_filterPushDown_factToRegionToCountryLeftFilterNormalizedAlreadyPushDownVariety() @@ -581,14 +562,31 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterNormalizedAlready regionToCountry(JoinType.LEFT) ); - JoinFilterPreAnalysis joinFilterPreAnalysis = simplePreAnalysis( - joinableClauses, - originalFilter - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - joinFilterPreAnalysis + joinFilterPreAnalysisGroup + ); + + JoinTestHelper.verifyCursors( + adapter.makeCursors( + originalFilter, + Intervals.ETERNITY, + VirtualColumns.EMPTY, + Granularities.ALL, + false, + null + ), + ImmutableList.of( + "page", + FACT_TO_REGION_PREFIX + "regionName", + REGION_TO_COUNTRY_PREFIX + "countryName" + ), + ImmutableList.of( + new Object[]{"Les Argonautes", "Quebec", "Canada"} + ) ); JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( @@ -655,27 +653,9 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterNormalizedAlready ), ImmutableSet.of() ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); - - JoinTestHelper.verifyCursors( - adapter.makeCursors( - originalFilter, - Intervals.ETERNITY, - VirtualColumns.EMPTY, - Granularities.ALL, - false, - null - ), - ImmutableList.of( - "page", - FACT_TO_REGION_PREFIX + "regionName", - REGION_TO_COUNTRY_PREFIX + "countryName" - ), - ImmutableList.of( - new Object[]{"Les Argonautes", "Quebec", "Canada"} - ) - ); } @Test @@ -705,15 +685,32 @@ public void test_filterPushDown_factExpressionsToRegionToCountryLeftFilterOnChan new SelectorFilter("rtc.countryName", "States United") ) ); - JoinFilterPreAnalysis joinFilterPreAnalysis = simplePreAnalysis( - joinableClauses, - originalFilter - ); + + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - joinFilterPreAnalysis + joinFilterPreAnalysisGroup + ); + + JoinTestHelper.verifyCursors( + adapter.makeCursors( + originalFilter, + Intervals.ETERNITY, + VirtualColumns.EMPTY, + Granularities.ALL, + false, + null + ), + ImmutableList.of( + "page", + FACT_TO_REGION_PREFIX + "regionName", + REGION_TO_COUNTRY_PREFIX + "countryName" + ), + ImmutableList.of( + new Object[]{"Old Anatolian Turkish", "Ainigriv", "States United"} + ) ); JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( @@ -726,6 +723,7 @@ public void test_filterPushDown_factExpressionsToRegionToCountryLeftFilterOnChan new SelectorFilter("rtc.countryName", "States United"), ImmutableSet.of() ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); ExpressionVirtualColumn expectedVirtualColumn = new ExpressionVirtualColumn( @@ -745,25 +743,6 @@ public void test_filterPushDown_factExpressionsToRegionToCountryLeftFilterOnChan ExpressionVirtualColumn actualVirtualColumn = (ExpressionVirtualColumn) actualFilterSplit.getPushDownVirtualColumns() .iterator().next(); compareExpressionVirtualColumns(expectedVirtualColumn, actualVirtualColumn); - - JoinTestHelper.verifyCursors( - adapter.makeCursors( - originalFilter, - Intervals.ETERNITY, - VirtualColumns.EMPTY, - Granularities.ALL, - false, - null - ), - ImmutableList.of( - "page", - FACT_TO_REGION_PREFIX + "regionName", - REGION_TO_COUNTRY_PREFIX + "countryName" - ), - ImmutableList.of( - new Object[]{"Old Anatolian Turkish", "Ainigriv", "States United"} - ) - ); } @Test @@ -795,17 +774,15 @@ public void test_filterPushDown_factToRegionToCountryNotEquiJoinLeftFilterOnChan ) ); - JoinFilterPreAnalysis joinFilterPreAnalysis = simplePreAnalysis( - joinableClauses, - originalFilter - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("Cannot build hash-join matcher on non-equi-join condition: \"r1.regionIsoCode\" == regionIsoCode && reverse(\"r1.countryIsoCode\") == countryIsoCode"); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - joinFilterPreAnalysis + joinFilterPreAnalysisGroup ); JoinTestHelper.verifyCursors( adapter.makeCursors( @@ -846,14 +823,32 @@ public void test_filterPushDown_factToRegionToCountryLeftUnnormalizedFilter() ) ); - JoinFilterPreAnalysis joinFilterPreAnalysis = simplePreAnalysis( - joinableClauses, - originalFilter - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - joinFilterPreAnalysis + joinFilterPreAnalysisGroup + ); + + JoinTestHelper.verifyCursors( + adapter.makeCursors( + originalFilter, + Intervals.ETERNITY, + VirtualColumns.EMPTY, + Granularities.ALL, + false, + null + ), + ImmutableList.of( + "page", + FACT_TO_REGION_PREFIX + "regionName", + REGION_TO_COUNTRY_PREFIX + "countryName" + ), + ImmutableList.of( + new Object[]{"유희왕 GX", "Seoul", "Republic of Korea"}, + new Object[]{"Old Anatolian Turkish", "Virginia", "United States"} + ) ); JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( @@ -896,28 +891,9 @@ public void test_filterPushDown_factToRegionToCountryLeftUnnormalizedFilter() ), ImmutableSet.of() ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); - - JoinTestHelper.verifyCursors( - adapter.makeCursors( - originalFilter, - Intervals.ETERNITY, - VirtualColumns.EMPTY, - Granularities.ALL, - false, - null - ), - ImmutableList.of( - "page", - FACT_TO_REGION_PREFIX + "regionName", - REGION_TO_COUNTRY_PREFIX + "countryName" - ), - ImmutableList.of( - new Object[]{"유희왕 GX", "Seoul", "Republic of Korea"}, - new Object[]{"Old Anatolian Turkish", "Virginia", "United States"} - ) - ); } @Test @@ -946,15 +922,32 @@ public void test_filterPushDown_factConcatExpressionToCountryLeftFilterOnChannel ) ); - JoinFilterPreAnalysis joinFilterPreAnalysis = simplePreAnalysis( - joinableClauses, - filter - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - joinFilterPreAnalysis + joinFilterPreAnalysisGroup + ); + + JoinTestHelper.verifyCursors( + adapter.makeCursors( + filter, + Intervals.ETERNITY, + VirtualColumns.EMPTY, + Granularities.ALL, + false, + null + ), + ImmutableList.of( + "page", + FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "countryName" + ), + ImmutableList.of( + new Object[]{"President of India", "Usca"}, + new Object[]{"Otjiwarongo Airport", "Usca"}, + new Object[]{"Carlo Curti", "Usca"} + ) ); ExpressionVirtualColumn expectedVirtualColumn = new ExpressionVirtualColumn( @@ -975,6 +968,7 @@ public void test_filterPushDown_factConcatExpressionToCountryLeftFilterOnChannel expectedVirtualColumn ) ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(filter); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals( expectedFilterSplit.getBaseTableFilter(), @@ -987,26 +981,6 @@ public void test_filterPushDown_factConcatExpressionToCountryLeftFilterOnChannel ExpressionVirtualColumn actualVirtualColumn = (ExpressionVirtualColumn) actualFilterSplit.getPushDownVirtualColumns() .iterator().next(); compareExpressionVirtualColumns(expectedVirtualColumn, actualVirtualColumn); - - JoinTestHelper.verifyCursors( - adapter.makeCursors( - filter, - Intervals.ETERNITY, - VirtualColumns.EMPTY, - Granularities.ALL, - false, - null - ), - ImmutableList.of( - "page", - FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "countryName" - ), - ImmutableList.of( - new Object[]{"President of India", "Usca"}, - new Object[]{"Otjiwarongo Airport", "Usca"}, - new Object[]{"Carlo Curti", "Usca"} - ) - ); } @Test @@ -1035,15 +1009,32 @@ public void test_filterPushDown_factConcatExpressionToCountryLeftFilterOnChannel ) ); - JoinFilterPreAnalysis joinFilterPreAnalysis = simplePreAnalysis( - joinableClauses, - filter - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - joinFilterPreAnalysis + joinFilterPreAnalysisGroup + ); + + JoinTestHelper.verifyCursors( + adapter.makeCursors( + filter, + Intervals.ETERNITY, + VirtualColumns.EMPTY, + Granularities.ALL, + false, + null + ), + ImmutableList.of( + "page", + FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "v" + ), + ImmutableList.of( + new Object[]{"President of India", "Usca"}, + new Object[]{"Otjiwarongo Airport", "Usca"}, + new Object[]{"Carlo Curti", "Usca"} + ) ); ExpressionVirtualColumn expectedVirtualColumn = new ExpressionVirtualColumn( @@ -1064,6 +1055,7 @@ public void test_filterPushDown_factConcatExpressionToCountryLeftFilterOnChannel expectedVirtualColumn ) ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(filter); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals( expectedFilterSplit.getBaseTableFilter(), @@ -1076,26 +1068,6 @@ public void test_filterPushDown_factConcatExpressionToCountryLeftFilterOnChannel ExpressionVirtualColumn actualVirtualColumn = (ExpressionVirtualColumn) actualFilterSplit.getPushDownVirtualColumns() .iterator().next(); compareExpressionVirtualColumns(expectedVirtualColumn, actualVirtualColumn); - - JoinTestHelper.verifyCursors( - adapter.makeCursors( - filter, - Intervals.ETERNITY, - VirtualColumns.EMPTY, - Granularities.ALL, - false, - null - ), - ImmutableList.of( - "page", - FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "v" - ), - ImmutableList.of( - new Object[]{"President of India", "Usca"}, - new Object[]{"Otjiwarongo Airport", "Usca"}, - new Object[]{"Carlo Curti", "Usca"} - ) - ); } @Test @@ -1108,29 +1080,13 @@ public void test_filterPushDown_factToCountryRightWithFilterOnChannelAndJoinable new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "countryName", "Germany") ) ); - JoinFilterPreAnalysis joinFilterPreAnalysis = simplePreAnalysis( - joinableClauses, - originalFilter - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - joinFilterPreAnalysis + joinFilterPreAnalysisGroup ); - JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( - new AndFilter( - ImmutableList.of( - new SelectorFilter("channel", "#de.wikipedia"), - new InDimFilter("countryIsoCode", ImmutableSet.of("DE"), null, null).toFilter() - ) - ), - new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "countryName", "Germany"), - ImmutableSet.of() - ); - JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); - Assert.assertEquals(expectedFilterSplit, actualFilterSplit); - JoinTestHelper.verifyCursors( adapter.makeCursors( originalFilter, @@ -1152,6 +1108,20 @@ public void test_filterPushDown_factToCountryRightWithFilterOnChannelAndJoinable new Object[]{"Diskussion:Sebastian Schulz", "DE", 3L, "DE", "Germany", 3L} ) ); + + JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( + new AndFilter( + ImmutableList.of( + new SelectorFilter("channel", "#de.wikipedia"), + new InDimFilter("countryIsoCode", ImmutableSet.of("DE"), null, null).toFilter() + ) + ), + new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "countryName", "Germany"), + ImmutableSet.of() + ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); + Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @Test @@ -1164,28 +1134,13 @@ public void test_filterPushDown_factToCountryRightWithFilterOnChannelAndJoinable new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "v", "Germany") ) ); - JoinFilterPreAnalysis joinFilterPreAnalysis = simplePreAnalysis( - joinableClauses, - originalFilter - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - joinFilterPreAnalysis - ); - - JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( - new AndFilter( - ImmutableList.of( - new SelectorFilter("channel", "#de.wikipedia"), - new InDimFilter("countryIsoCode", ImmutableSet.of("DE"), null, null).toFilter() - ) - ), - new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "v", "Germany"), - ImmutableSet.of() + joinFilterPreAnalysisGroup ); - JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); - Assert.assertEquals(expectedFilterSplit, actualFilterSplit); JoinTestHelper.verifyCursors( adapter.makeCursors( @@ -1207,6 +1162,20 @@ public void test_filterPushDown_factToCountryRightWithFilterOnChannelAndJoinable new Object[]{"Diskussion:Sebastian Schulz", "DE", 3L, "DE", "Germany"} ) ); + + JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( + new AndFilter( + ImmutableList.of( + new SelectorFilter("channel", "#de.wikipedia"), + new InDimFilter("countryIsoCode", ImmutableSet.of("DE"), null, null).toFilter() + ) + ), + new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "v", "Germany"), + ImmutableSet.of() + ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); + Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @Test @@ -1219,29 +1188,14 @@ public void test_filterPushDown_factToCountryRightWithFilterOnNullColumns() new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "countryName", null) ) ); - JoinFilterPreAnalysis joinFilterPreAnalysis = simplePreAnalysis( - joinableClauses, - originalFilter - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - joinFilterPreAnalysis + joinFilterPreAnalysisGroup ); - JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( - null, - new AndFilter( - ImmutableList.of( - new SelectorFilter("channel", null), - new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "countryName", null) - ) - ), - ImmutableSet.of() - ); - JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); - Assert.assertEquals(expectedFilterSplit, actualFilterSplit); - JoinTestHelper.verifyCursors( adapter.makeCursors( originalFilter, @@ -1261,6 +1215,20 @@ public void test_filterPushDown_factToCountryRightWithFilterOnNullColumns() ), ImmutableList.of() ); + + JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( + null, + new AndFilter( + ImmutableList.of( + new SelectorFilter("channel", null), + new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "countryName", null) + ) + ), + ImmutableSet.of() + ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); + Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @Test @@ -1273,29 +1241,14 @@ public void test_filterPushDown_factToCountryRightWithFilterOnValueThatMatchesNo new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "countryName", "NO MATCH") ) ); - JoinFilterPreAnalysis joinFilterPreAnalysis = simplePreAnalysis( - joinableClauses, - originalFilter - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - joinFilterPreAnalysis + joinFilterPreAnalysisGroup ); - JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( - FalseFilter.instance(), - new AndFilter( - ImmutableList.of( - new SelectorFilter("channel", null), - new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "countryName", "NO MATCH") - ) - ), - ImmutableSet.of() - ); - JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); - Assert.assertEquals(expectedFilterSplit, actualFilterSplit); - JoinTestHelper.verifyCursors( adapter.makeCursors( originalFilter, @@ -1315,6 +1268,21 @@ public void test_filterPushDown_factToCountryRightWithFilterOnValueThatMatchesNo ), ImmutableList.of() ); + + + JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( + FalseFilter.instance(), + new AndFilter( + ImmutableList.of( + new SelectorFilter("channel", null), + new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "countryName", "NO MATCH") + ) + ), + ImmutableSet.of() + ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); + Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @Test @@ -1327,28 +1295,13 @@ public void test_filterPushDown_factToCountryRightWithFilterOnNullColumnsUsingLo new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "v", null) ) ); - JoinFilterPreAnalysis joinFilterPreAnalysis = simplePreAnalysis( - joinableClauses, - originalFilter - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - joinFilterPreAnalysis - ); - - JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( - null, - new AndFilter( - ImmutableList.of( - new SelectorFilter("channel", null), - new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "v", null) - ) - ), - ImmutableSet.of() + joinFilterPreAnalysisGroup ); - JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); - Assert.assertEquals(expectedFilterSplit, actualFilterSplit); JoinTestHelper.verifyCursors( adapter.makeCursors( @@ -1368,6 +1321,20 @@ public void test_filterPushDown_factToCountryRightWithFilterOnNullColumnsUsingLo ), ImmutableList.of() ); + + JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( + null, + new AndFilter( + ImmutableList.of( + new SelectorFilter("channel", null), + new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "v", null) + ) + ), + ImmutableSet.of() + ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); + Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @Test @@ -1380,29 +1347,14 @@ public void test_filterPushDown_factToCountryInnerUsingCountryNumberFilterOnChan new SelectorFilter(FACT_TO_COUNTRY_ON_NUMBER_PREFIX + "countryName", "Australia") ) ); - JoinFilterPreAnalysis joinFilterPreAnalysis = simplePreAnalysis( - joinableClauses, - originalFilter - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - joinFilterPreAnalysis + joinFilterPreAnalysisGroup ); - JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( - new AndFilter( - ImmutableList.of( - new SelectorFilter("channel", "#en.wikipedia"), - new InDimFilter("countryNumber", ImmutableSet.of("0"), null, null).toFilter() - ) - ), - new SelectorFilter(FACT_TO_COUNTRY_ON_NUMBER_PREFIX + "countryName", "Australia"), - ImmutableSet.of() - ); - JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); - Assert.assertEquals(expectedFilterSplit, actualFilterSplit); - // In non-SQL-compatible mode, we get an extra row, since the 'null' countryNumber for "Talk:Oswald Tilghman" // is interpreted as 0 (a.k.a. Australia). JoinTestHelper.verifyCursors( @@ -1430,6 +1382,20 @@ public void test_filterPushDown_factToCountryInnerUsingCountryNumberFilterOnChan new Object[]{"Peremptory norm", "AU", "AU", "Australia", 0L} ) ); + + JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( + new AndFilter( + ImmutableList.of( + new SelectorFilter("channel", "#en.wikipedia"), + new InDimFilter("countryNumber", ImmutableSet.of("0"), null, null).toFilter() + ) + ), + new SelectorFilter(FACT_TO_COUNTRY_ON_NUMBER_PREFIX + "countryName", "Australia"), + ImmutableSet.of() + ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); + Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @Test @@ -1442,28 +1408,13 @@ public void test_filterPushDown_factToCountryInnerUsingCountryNumberFilterOnChan new SelectorFilter(FACT_TO_COUNTRY_ON_NUMBER_PREFIX + "v", "Australia") ) ); - JoinFilterPreAnalysis joinFilterPreAnalysis = simplePreAnalysis( - joinableClauses, - originalFilter - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - joinFilterPreAnalysis - ); - - JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( - new AndFilter( - ImmutableList.of( - new SelectorFilter("channel", "#en.wikipedia"), - new InDimFilter("countryNumber", ImmutableSet.of("0"), null, null).toFilter() - ) - ), - new SelectorFilter(FACT_TO_COUNTRY_ON_NUMBER_PREFIX + "v", "Australia"), - ImmutableSet.of() + joinFilterPreAnalysisGroup ); - JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); - Assert.assertEquals(expectedFilterSplit, actualFilterSplit); // In non-SQL-compatible mode, we get an extra row, since the 'null' countryNumber for "Talk:Oswald Tilghman" // is interpreted as 0 (a.k.a. Australia). @@ -1491,6 +1442,20 @@ public void test_filterPushDown_factToCountryInnerUsingCountryNumberFilterOnChan new Object[]{"Peremptory norm", "AU", "0", "Australia"} ) ); + + JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( + new AndFilter( + ImmutableList.of( + new SelectorFilter("channel", "#en.wikipedia"), + new InDimFilter("countryNumber", ImmutableSet.of("0"), null, null).toFilter() + ) + ), + new SelectorFilter(FACT_TO_COUNTRY_ON_NUMBER_PREFIX + "v", "Australia"), + ImmutableSet.of() + ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); + Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @Test @@ -1503,29 +1468,14 @@ public void test_filterPushDown_factToCountryInnerUsingCountryNumberFilterOnNull new SelectorFilter(FACT_TO_COUNTRY_ON_NUMBER_PREFIX + "countryName", null) ) ); - JoinFilterPreAnalysis joinFilterPreAnalysis = simplePreAnalysis( - joinableClauses, - originalFilter - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - joinFilterPreAnalysis + joinFilterPreAnalysisGroup ); - JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( - null, - new AndFilter( - ImmutableList.of( - new SelectorFilter("channel", null), - new SelectorFilter(FACT_TO_COUNTRY_ON_NUMBER_PREFIX + "countryName", null) - ) - ), - ImmutableSet.of() - ); - JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); - Assert.assertEquals(expectedFilterSplit, actualFilterSplit); - JoinTestHelper.verifyCursors( adapter.makeCursors( originalFilter, @@ -1544,6 +1494,20 @@ public void test_filterPushDown_factToCountryInnerUsingCountryNumberFilterOnNull ), ImmutableList.of() ); + + JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( + null, + new AndFilter( + ImmutableList.of( + new SelectorFilter("channel", null), + new SelectorFilter(FACT_TO_COUNTRY_ON_NUMBER_PREFIX + "countryName", null) + ) + ), + ImmutableSet.of() + ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); + Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @Test @@ -1556,28 +1520,13 @@ public void test_filterPushDown_factToCountryInnerUsingCountryNumberFilterOnNull new SelectorFilter(FACT_TO_COUNTRY_ON_NUMBER_PREFIX + "v", null) ) ); - JoinFilterPreAnalysis joinFilterPreAnalysis = simplePreAnalysis( - joinableClauses, - originalFilter - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - joinFilterPreAnalysis - ); - - JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( - null, - new AndFilter( - ImmutableList.of( - new SelectorFilter("channel", null), - new SelectorFilter(FACT_TO_COUNTRY_ON_NUMBER_PREFIX + "v", null) - ) - ), - ImmutableSet.of() + joinFilterPreAnalysisGroup ); - JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); - Assert.assertEquals(expectedFilterSplit, actualFilterSplit); JoinTestHelper.verifyCursors( adapter.makeCursors( @@ -1596,6 +1545,20 @@ public void test_filterPushDown_factToCountryInnerUsingCountryNumberFilterOnNull ), ImmutableList.of() ); + + JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( + null, + new AndFilter( + ImmutableList.of( + new SelectorFilter("channel", null), + new SelectorFilter(FACT_TO_COUNTRY_ON_NUMBER_PREFIX + "v", null) + ) + ), + ImmutableSet.of() + ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); + Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @Test @@ -1608,29 +1571,14 @@ public void test_filterPushDown_factToCountryFullWithFilterOnChannelAndCountryNa ) ); List joinableClauses = ImmutableList.of(factToCountryOnIsoCode(JoinType.FULL)); - JoinFilterPreAnalysis joinFilterPreAnalysis = simplePreAnalysis( - joinableClauses, - filter - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - joinFilterPreAnalysis + joinFilterPreAnalysisGroup ); - JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( - new AndFilter( - ImmutableList.of( - new SelectorFilter("channel", "#es.wikipedia"), - new InDimFilter("countryIsoCode", ImmutableSet.of("SV"), null, null).toFilter() - ) - ), - new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "countryName", "El Salvador"), - ImmutableSet.of() - ); - JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); - Assert.assertEquals(expectedFilterSplit, actualFilterSplit); - JoinTestHelper.verifyCursors( adapter.makeCursors( filter, @@ -1652,6 +1600,20 @@ public void test_filterPushDown_factToCountryFullWithFilterOnChannelAndCountryNa new Object[]{"Wendigo", "SV", 12L, "SV", "El Salvador", 12L} ) ); + + JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( + new AndFilter( + ImmutableList.of( + new SelectorFilter("channel", "#es.wikipedia"), + new InDimFilter("countryIsoCode", ImmutableSet.of("SV"), null, null).toFilter() + ) + ), + new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "countryName", "El Salvador"), + ImmutableSet.of() + ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(filter); + JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); + Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @Test @@ -1664,28 +1626,13 @@ public void test_filterPushDown_factToCountryFullWithFilterOnChannelAndCountryNa ) ); List joinableClauses = ImmutableList.of(factToCountryNameUsingIsoCodeLookup(JoinType.FULL)); - JoinFilterPreAnalysis joinFilterPreAnalysis = simplePreAnalysis( - joinableClauses, - filter - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - joinFilterPreAnalysis - ); - - JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( - new AndFilter( - ImmutableList.of( - new SelectorFilter("channel", "#es.wikipedia"), - new InDimFilter("countryIsoCode", ImmutableSet.of("SV"), null, null).toFilter() - ) - ), - new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "v", "El Salvador"), - ImmutableSet.of() + joinFilterPreAnalysisGroup ); - JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); - Assert.assertEquals(expectedFilterSplit, actualFilterSplit); JoinTestHelper.verifyCursors( adapter.makeCursors( @@ -1707,6 +1654,20 @@ public void test_filterPushDown_factToCountryFullWithFilterOnChannelAndCountryNa new Object[]{"Wendigo", "SV", 12L, "SV", "El Salvador"} ) ); + + JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( + new AndFilter( + ImmutableList.of( + new SelectorFilter("channel", "#es.wikipedia"), + new InDimFilter("countryIsoCode", ImmutableSet.of("SV"), null, null).toFilter() + ) + ), + new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "v", "El Salvador"), + ImmutableSet.of() + ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(filter); + JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); + Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @Test @@ -1719,29 +1680,14 @@ public void test_filterPushDown_factToCountryFullWithFilterOnNulls() new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "countryName", null) ) ); - JoinFilterPreAnalysis joinFilterPreAnalysis = simplePreAnalysis( - joinableClauses, - originalFilter - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - joinFilterPreAnalysis + joinFilterPreAnalysisGroup ); - JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( - null, - new AndFilter( - ImmutableList.of( - new SelectorFilter("channel", null), - new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "countryName", null) - ) - ), - ImmutableSet.of() - ); - JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); - Assert.assertEquals(expectedFilterSplit, actualFilterSplit); - JoinTestHelper.verifyCursors( adapter.makeCursors( originalFilter, @@ -1761,6 +1707,20 @@ public void test_filterPushDown_factToCountryFullWithFilterOnNulls() ), ImmutableList.of() ); + + JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( + null, + new AndFilter( + ImmutableList.of( + new SelectorFilter("channel", null), + new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "countryName", null) + ) + ), + ImmutableSet.of() + ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); + Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @Test @@ -1773,28 +1733,12 @@ public void test_filterPushDown_factToCountryFullWithFilterOnNullsUsingLookup() new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "v", null) ) ); - JoinFilterPreAnalysis joinFilterPreAnalysis = simplePreAnalysis( - joinableClauses, - originalFilter - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - joinFilterPreAnalysis - ); - - JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( - null, - new AndFilter( - ImmutableList.of( - new SelectorFilter("channel", null), - new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "v", null) - ) - ), - ImmutableSet.of() + joinFilterPreAnalysisGroup ); - JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); - Assert.assertEquals(expectedFilterSplit, actualFilterSplit); JoinTestHelper.verifyCursors( adapter.makeCursors( @@ -1814,6 +1758,20 @@ public void test_filterPushDown_factToCountryFullWithFilterOnNullsUsingLookup() ), ImmutableList.of() ); + + JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( + null, + new AndFilter( + ImmutableList.of( + new SelectorFilter("channel", null), + new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "v", null) + ) + ), + ImmutableSet.of() + ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); + Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @Test @@ -1838,29 +1796,13 @@ public void test_filterPushDown_factToRegionTwoColumnsToOneRHSColumnAndFilterOnR ); Filter originalFilter = new SelectorFilter("r1.regionName", "Fourems Province"); - JoinFilterPreAnalysis joinFilterPreAnalysis = simplePreAnalysis( - joinableClauses, - originalFilter - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - joinFilterPreAnalysis - ); - - JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( - new AndFilter( - ImmutableList.of( - new InDimFilter("countryIsoCode", ImmutableSet.of("MMMM"), null, null).toFilter(), - new InDimFilter("regionIsoCode", ImmutableSet.of("MMMM"), null, null).toFilter() - ) - ), - new SelectorFilter("r1.regionName", "Fourems Province"), - ImmutableSet.of() + joinFilterPreAnalysisGroup ); - JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); - Assert.assertEquals(expectedFilterSplit, actualFilterSplit); JoinTestHelper.verifyCursors( adapter.makeCursors( @@ -1879,6 +1821,20 @@ public void test_filterPushDown_factToRegionTwoColumnsToOneRHSColumnAndFilterOnR new Object[]{"History of Fourems", "Fourems Province"} ) ); + + JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( + new AndFilter( + ImmutableList.of( + new InDimFilter("countryIsoCode", ImmutableSet.of("MMMM"), null, null).toFilter(), + new InDimFilter("regionIsoCode", ImmutableSet.of("MMMM"), null, null).toFilter() + ) + ), + new SelectorFilter("r1.regionName", "Fourems Province"), + ImmutableSet.of() + ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); + Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @Test @@ -1908,28 +1864,13 @@ public void test_filterPushDown_factToRegionOneColumnToTwoRHSColumnsAndFilterOnR ) ); - JoinFilterPreAnalysis joinFilterPreAnalysis = simplePreAnalysis( - joinableClauses, - originalFilter - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - joinFilterPreAnalysis - ); - - JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( - new OrFilter( - ImmutableList.of( - new InDimFilter("regionIsoCode", ImmutableSet.of("MMMM"), null, null).toFilter(), - new SelectorFilter("regionIsoCode", "AAAA") - ) - ), - originalFilter, - ImmutableSet.of() + joinFilterPreAnalysisGroup ); - JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); - Assert.assertEquals(expectedFilterSplit, actualFilterSplit); JoinTestHelper.verifyCursors( adapter.makeCursors( @@ -1948,6 +1889,20 @@ public void test_filterPushDown_factToRegionOneColumnToTwoRHSColumnsAndFilterOnR new Object[]{"History of Fourems", "Fourems Province"} ) ); + + JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( + new OrFilter( + ImmutableList.of( + new InDimFilter("regionIsoCode", ImmutableSet.of("MMMM"), null, null).toFilter(), + new SelectorFilter("regionIsoCode", "AAAA") + ) + ), + originalFilter, + ImmutableSet.of() + ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); + Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @@ -1979,28 +1934,13 @@ public void test_filterPushDown_factToRegionThreeRHSColumnsAllDirectAndFilterOnR ) ); - JoinFilterPreAnalysis joinFilterPreAnalysis = simplePreAnalysis( - joinableClauses, - originalFilter - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - joinFilterPreAnalysis - ); - - JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( - new OrFilter( - ImmutableList.of( - new SelectorFilter("user", "Fourems Province"), - new SelectorFilter("regionIsoCode", "AAAA") - ) - ), - null, - ImmutableSet.of() + joinFilterPreAnalysisGroup ); - JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); - Assert.assertEquals(expectedFilterSplit, actualFilterSplit); // This query doesn't execute because regionName is not a key column, but we can still check the // filter rewrites. @@ -2024,8 +1964,21 @@ public void test_filterPushDown_factToRegionThreeRHSColumnsAllDirectAndFilterOnR ), ImmutableList.of() ); - } + JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( + new OrFilter( + ImmutableList.of( + new SelectorFilter("user", "Fourems Province"), + new SelectorFilter("regionIsoCode", "AAAA") + ) + ), + null, + ImmutableSet.of() + ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); + Assert.assertEquals(expectedFilterSplit, actualFilterSplit); + } @Test public void test_filterPushDown_factToRegionToCountryLeftFilterOnPageDisablePushDown() @@ -2036,28 +1989,18 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterOnPageDisablePush )); Filter originalFilter = new SelectorFilter("page", "Peremptory norm"); - JoinFilterPreAnalysis joinFilterPreAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - joinableClauses, - VirtualColumns.EMPTY, - originalFilter, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( false, true, true, QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE ); + HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses.getJoinableClauses(), - joinFilterPreAnalysis - ); - - JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( - null, - new SelectorFilter("page", "Peremptory norm"), - ImmutableSet.of() + joinFilterPreAnalysisGroup ); - JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); - Assert.assertEquals(expectedFilterSplit, actualFilterSplit); JoinTestHelper.verifyCursors( adapter.makeCursors( @@ -2077,6 +2020,15 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterOnPageDisablePush new Object[]{"Peremptory norm", "New South Wales", "Australia"} ) ); + + JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( + null, + new SelectorFilter("page", "Peremptory norm"), + ImmutableSet.of() + ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); + Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @Test @@ -2108,10 +2060,7 @@ public void test_filterPushDown_factToRegionToCountryLeftEnablePushDownDisableRe )) ) ); - JoinFilterPreAnalysis joinFilterPreAnalysis = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - joinableClauses, - VirtualColumns.EMPTY, - originalFilter, + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( true, false, true, @@ -2120,7 +2069,30 @@ public void test_filterPushDown_factToRegionToCountryLeftEnablePushDownDisableRe HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses.getJoinableClauses(), - joinFilterPreAnalysis + joinFilterPreAnalysisGroup + ); + + JoinTestHelper.verifyCursors( + adapter.makeCursors( + originalFilter, + Intervals.ETERNITY, + VirtualColumns.EMPTY, + Granularities.ALL, + false, + null + ), + ImmutableList.of( + "page", + FACT_TO_REGION_PREFIX + "regionName", + REGION_TO_COUNTRY_PREFIX + "countryName" + ), + ImmutableList.of( + new Object[]{"President of India", "California", "United States"}, + new Object[]{"Otjiwarongo Airport", "California", "United States"}, + new Object[]{"DirecTV", "North Carolina", "United States"}, + new Object[]{"Carlo Curti", "California", "United States"}, + new Object[]{"Old Anatolian Turkish", "Virginia", "United States"} + ) ); JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( @@ -2152,31 +2124,9 @@ public void test_filterPushDown_factToRegionToCountryLeftEnablePushDownDisableRe ), ImmutableSet.of() ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); - - JoinTestHelper.verifyCursors( - adapter.makeCursors( - originalFilter, - Intervals.ETERNITY, - VirtualColumns.EMPTY, - Granularities.ALL, - false, - null - ), - ImmutableList.of( - "page", - FACT_TO_REGION_PREFIX + "regionName", - REGION_TO_COUNTRY_PREFIX + "countryName" - ), - ImmutableList.of( - new Object[]{"President of India", "California", "United States"}, - new Object[]{"Otjiwarongo Airport", "California", "United States"}, - new Object[]{"DirecTV", "North Carolina", "United States"}, - new Object[]{"Carlo Curti", "California", "United States"}, - new Object[]{"Old Anatolian Turkish", "Virginia", "United States"} - ) - ); } @Test @@ -2284,15 +2234,11 @@ public boolean supportsRequiredColumnRewrite() regionToCountry(JoinType.LEFT) ); - JoinFilterPreAnalysis joinFilterPreAnalysis = simplePreAnalysis( - joinableClauses, - originalFilter - ); - + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - joinFilterPreAnalysis + joinFilterPreAnalysisGroup ); String rewrittenCountryIsoCodeColumnName = hasLhsExpressionInJoinCondition @@ -2324,6 +2270,25 @@ public boolean supportsRequiredColumnRewrite() expectedVirtualColumns = ImmutableSet.of(); } + JoinTestHelper.verifyCursors( + adapter.makeCursors( + originalFilter, + Intervals.ETERNITY, + VirtualColumns.EMPTY, + Granularities.ALL, + false, + null + ), + ImmutableList.of( + "page", + FACT_TO_REGION_PREFIX + "regionName", + REGION_TO_COUNTRY_PREFIX + "countryName" + ), + ImmutableList.of( + new Object[]{"Didier Leclair", "Ontario", "Canada"} + ) + ); + JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( new AndFilter( ImmutableList.of( @@ -2401,27 +2366,9 @@ public boolean supportsRequiredColumnRewrite() ), expectedVirtualColumns ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); - - JoinTestHelper.verifyCursors( - adapter.makeCursors( - originalFilter, - Intervals.ETERNITY, - VirtualColumns.EMPTY, - Granularities.ALL, - false, - null - ), - ImmutableList.of( - "page", - FACT_TO_REGION_PREFIX + "regionName", - REGION_TO_COUNTRY_PREFIX + "countryName" - ), - ImmutableList.of( - new Object[]{"Didier Leclair", "Ontario", "Canada"} - ) - ); } @Test @@ -2439,14 +2386,30 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterOnTwoRHSColumnsSa regionToCountry(JoinType.LEFT) ); - JoinFilterPreAnalysis joinFilterPreAnalysis = simplePreAnalysis( - joinableClauses, - originalFilter - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, - joinFilterPreAnalysis + joinFilterPreAnalysisGroup + ); + + JoinTestHelper.verifyCursors( + adapter.makeCursors( + originalFilter, + Intervals.ETERNITY, + VirtualColumns.EMPTY, + Granularities.ALL, + false, + null + ), + ImmutableList.of( + "page", + FACT_TO_REGION_PREFIX + "regionName", + REGION_TO_COUNTRY_PREFIX + "countryName" + ), + ImmutableList.of( + ) ); JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( @@ -2469,26 +2432,9 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterOnTwoRHSColumnsSa originalFilter, ImmutableSet.of() ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); - - JoinTestHelper.verifyCursors( - adapter.makeCursors( - originalFilter, - Intervals.ETERNITY, - VirtualColumns.EMPTY, - Granularities.ALL, - false, - null - ), - ImmutableList.of( - "page", - FACT_TO_REGION_PREFIX + "regionName", - REGION_TO_COUNTRY_PREFIX + "countryName" - ), - ImmutableList.of( - ) - ); } @Test @@ -2500,19 +2446,14 @@ public void test_JoinFilterSplit_equals() .verify(); } - private static JoinFilterPreAnalysis simplePreAnalysis( - List joinableClauses, - Filter originalFilter - ) + private static JoinFilterPreAnalysisGroup simplePreAnalysisGroup() { - return JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - JoinableClauses.fromList(joinableClauses), - VirtualColumns.EMPTY, - originalFilter, - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + return new JoinFilterPreAnalysisGroup( + true, + true, + true, + QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE + ); + } } 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 94edd222dc67..5df585bc2b20 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 @@ -31,7 +31,6 @@ import org.apache.druid.common.config.NullHandling; import org.apache.druid.java.util.common.DateTimes; import org.apache.druid.java.util.common.IAE; -import org.apache.druid.java.util.common.ISE; import org.apache.druid.java.util.common.Intervals; import org.apache.druid.java.util.common.JodaUtils; import org.apache.druid.java.util.common.StringUtils; @@ -11486,79 +11485,75 @@ public void testTimeExtractWithTooFewArguments() throws Exception @Parameters(source = QueryContextForJoinProvider.class) public void testNestedGroupByOnInlineDataSourceWithFilterIsNotSupported(Map queryContext) throws Exception { - try { - testQuery( - "with abc as" - + "(" - + " SELECT dim1, m2 from druid.foo where \"__time\" >= '2001-01-02'" - + ")" - + ", def as" - + "(" - + " SELECT t1.dim1, SUM(t2.m2) as \"metricSum\" " - + " from abc as t1 inner join abc as t2 on t1.dim1 = t2.dim1" - + " where t1.dim1='def'" - + " group by 1" - + ")" - + "SELECT count(*) from def", - queryContext, - ImmutableList.of( - GroupByQuery - .builder() - .setDataSource( - 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", "m2") - .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())) - .build() - ) - .setGranularity(Granularities.ALL) - .setInterval(querySegmentSpec(Filtration.eternity())) - .build() - ), - ImmutableList.of(new Object[] {1}) - ); - Assert.fail("Expected an ISE to be thrown"); - } - catch (RuntimeException e) { - Throwable cause = e.getCause(); - boolean foundISE = false; - while (cause != null) { - if (cause instanceof ISE) { - foundISE = true; - break; - } - cause = cause.getCause(); - } - Assert.assertTrue(foundISE); - } + // Cannot vectorize due to virtual columns. + cannotVectorize(); + + testQuery( + "with abc as" + + "(" + + " SELECT dim1, m2 from druid.foo where \"__time\" >= '2001-01-02'" + + ")" + + ", def as" + + "(" + + " SELECT t1.dim1, SUM(t2.m2) as \"metricSum\" " + + " from abc as t1 inner join abc as t2 on t1.dim1 = t2.dim1" + + " where t1.dim1='def'" + + " group by 1" + + ")" + + "SELECT count(*) from def", + queryContext, + ImmutableList.of( + GroupByQuery + .builder() + .setDataSource( + 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(selector("dim1", "def", null)) + .setDimensions( + dimensions( + new DefaultDimensionSpec("v0", "d0") + ) + ) + .setVirtualColumns(expressionVirtualColumn("v0", "'def'", ValueType.STRING)) + .build() + ) + .setAggregatorSpecs(aggregators(new CountAggregatorFactory("a0"))) + .setGranularity(Granularities.ALL) + .setInterval(querySegmentSpec(Filtration.eternity())) + .build() + ), + ImmutableList.of(new Object[] {1L}) + ); } @Test From d3b30552efbe8ec69e3629b4990b6aa52ff5a5ab Mon Sep 17 00:00:00 2001 From: jon-wei Date: Wed, 3 Jun 2020 12:14:22 -0700 Subject: [PATCH 2/9] Add javadocs, fix inspections --- .../druid/segment/join/HashJoinSegment.java | 3 +- .../join/HashJoinSegmentStorageAdapter.java | 4 +-- .../apache/druid/segment/join/Joinables.java | 19 +++++------- .../join/filter/JoinFilterAnalyzer.java | 2 ++ .../join/filter/JoinFilterPreAnalysis.java | 14 ++------- .../rewrite/JoinFilterPreAnalysisGroup.java | 9 ++++++ .../HashJoinSegmentStorageAdapterTest.java | 30 ++++++++++--------- .../druid/segment/join/JoinablesTest.java | 13 ++------ .../appenderator/SinkQuerySegmentWalker.java | 4 +-- .../druid/server/LocalQuerySegmentWalker.java | 4 +-- .../server/coordination/ServerManager.java | 4 +-- .../server/TestClusterQuerySegmentWalker.java | 4 +-- 12 files changed, 47 insertions(+), 63 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegment.java b/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegment.java index 8a8c12933d2b..82640911f72b 100644 --- a/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegment.java +++ b/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegment.java @@ -47,7 +47,8 @@ public class HashJoinSegment extends AbstractSegment * @param baseSegment The left-hand side base segment * @param clauses The right-hand side clauses. The caller is responsible for ensuring that there are no * duplicate prefixes or prefixes that shadow each other across the clauses - * @param joinFilterPreAnalysisGroup Pre-analysis computed by {@link org.apache.druid.segment.join.filter.JoinFilterAnalyzer#computeJoinFilterPreAnalysis} + * @param joinFilterPreAnalysisGroup Pre-analysis group that holds all of the JoinFilterPreAnalysis results within + * the scope of a query */ public HashJoinSegment( Segment baseSegment, diff --git a/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapter.java b/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapter.java index 6e3ad7708d4f..891932424a3e 100644 --- a/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapter.java +++ b/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapter.java @@ -60,8 +60,8 @@ public class HashJoinSegmentStorageAdapter implements StorageAdapter /** * @param baseAdapter A StorageAdapter for the left-hand side base segment * @param clauses The right-hand side clauses. The caller is responsible for ensuring that there are no - * @param joinFilterPreAnalysisGroup - */ + * @param joinFilterPreAnalysisGroup Pre-analysis group that holds all of the JoinFilterPreAnalysis results within + * the scope of a query */ HashJoinSegmentStorageAdapter( StorageAdapter baseAdapter, List clauses, diff --git a/processing/src/main/java/org/apache/druid/segment/join/Joinables.java b/processing/src/main/java/org/apache/druid/segment/join/Joinables.java index b5376585d394..3feee09ab4c5 100644 --- a/processing/src/main/java/org/apache/druid/segment/join/Joinables.java +++ b/processing/src/main/java/org/apache/druid/segment/join/Joinables.java @@ -70,22 +70,19 @@ public static boolean isPrefixedBy(final String columnName, final String prefix) /** * Creates a Function that maps base segments to {@link HashJoinSegment} if needed (i.e. if the number of join * clauses is > 0). If mapping is not needed, this method will return {@link Function#identity()}. - * * @param clauses pre-joinable clauses * @param joinableFactory factory for joinables * @param cpuTimeAccumulator an accumulator that we will add CPU nanos to; this is part of the function to encourage - * callers to remember to track metrics on CPU time required for creation of Joinables + * callers to remember to track metrics on CPU time required for creation of Joinables * @param enableFilterPushDown whether to enable filter push down optimizations to the base segment. In production - * this should generally be {@code QueryContexts.getEnableJoinFilterPushDown(query)}. +* this should generally be {@code QueryContexts.getEnableJoinFilterPushDown(query)}. * @param enableFilterRewrite whether to enable filter rewrite optimizations for RHS columns. In production - * this should generally be {@code QueryContexts.getEnableJoinFilterRewrite(query)}. +* this should generally be {@code QueryContexts.getEnableJoinFilterRewrite(query)}. * @param enableRewriteValueColumnFilters whether to enable filter rewrite optimizations for RHS columns that are not - * key columns. In production this should generally - * be {@code QueryContexts.getEnableJoinFilterRewriteValueColumnFilters(query)}. +* key columns. In production this should generally +* be {@code QueryContexts.getEnableJoinFilterRewriteValueColumnFilters(query)}. * @param filterRewriteMaxSize the max allowed size of correlated value sets for RHS rewrites. In production - * this should generally be {@code QueryContexts.getJoinFilterRewriteMaxSize(query)}. - * @param originalFilter The original filter from the query. - * @param virtualColumns The virtual columns from the query. +* this should generally be {@code QueryContexts.getJoinFilterRewriteMaxSize(query)}. */ public static Function createSegmentMapFn( final List clauses, @@ -94,9 +91,7 @@ public static Function createSegmentMapFn( final boolean enableFilterPushDown, final boolean enableFilterRewrite, final boolean enableRewriteValueColumnFilters, - final long filterRewriteMaxSize, - final Filter originalFilter, - final VirtualColumns virtualColumns + final long filterRewriteMaxSize ) { // compute column correlations here and RHS correlated values diff --git a/processing/src/main/java/org/apache/druid/segment/join/filter/JoinFilterAnalyzer.java b/processing/src/main/java/org/apache/druid/segment/join/filter/JoinFilterAnalyzer.java index bdca7b91c8b1..91929a450b6d 100644 --- a/processing/src/main/java/org/apache/druid/segment/join/filter/JoinFilterAnalyzer.java +++ b/processing/src/main/java/org/apache/druid/segment/join/filter/JoinFilterAnalyzer.java @@ -83,6 +83,8 @@ public class JoinFilterAnalyzer * * See {@link JoinFilterPreAnalysis} for details on the result of this pre-analysis step. * + * @param joinFilterPreAnalysisGroup The query-scoped pre analysis group, used to hold pre-analysis results + * on a per-filter basis * @param joinableClauses The joinable clauses from the query * @param virtualColumns The virtual columns from the query * @param originalFilter The original filter from the query diff --git a/processing/src/main/java/org/apache/druid/segment/join/filter/JoinFilterPreAnalysis.java b/processing/src/main/java/org/apache/druid/segment/join/filter/JoinFilterPreAnalysis.java index cd04601786ab..da48f0536f26 100644 --- a/processing/src/main/java/org/apache/druid/segment/join/filter/JoinFilterPreAnalysis.java +++ b/processing/src/main/java/org/apache/druid/segment/join/filter/JoinFilterPreAnalysis.java @@ -39,11 +39,11 @@ * A JoinFilterPreAnalysis contains filter push down/rewrite information that does not have per-segment dependencies. * This includes: * - The query's JoinableClauses list - * - The query's original filter (if any) + * - The original filter that an analysis was performed ons * - A list of filter clauses from the original filter's CNF representation that only reference the base table * - A list of filter clauses from the original filter's CNF representation that reference RHS join tables * - A list of virtual columns that can only be computed post-join - * - Control flag booleans for whether filter push down and RHS rewrites are enabled. + * - The JoinFilterPreAnalysisGroup that this pre-analysis is associated with. */ public class JoinFilterPreAnalysis { @@ -122,16 +122,6 @@ public boolean isEnableFilterRewrite() return myGroup.isEnableFilterRewrite(); } - public boolean isEnableRewriteValueColumnFilters() - { - return myGroup.isEnableRewriteValueColumnFilters(); - } - - public long getFilterRewriteMaxSize() - { - return myGroup.getFilterRewriteMaxSize(); - } - public Equiconditions getEquiconditions() { return equiconditions; diff --git a/processing/src/main/java/org/apache/druid/segment/join/filter/rewrite/JoinFilterPreAnalysisGroup.java b/processing/src/main/java/org/apache/druid/segment/join/filter/rewrite/JoinFilterPreAnalysisGroup.java index f26f0dc732c3..0b3af2f61de3 100644 --- a/processing/src/main/java/org/apache/druid/segment/join/filter/rewrite/JoinFilterPreAnalysisGroup.java +++ b/processing/src/main/java/org/apache/druid/segment/join/filter/rewrite/JoinFilterPreAnalysisGroup.java @@ -24,6 +24,15 @@ import java.util.concurrent.ConcurrentHashMap; +/** + * A JoinFilterPreAnalysisGroup holds all of the JoinFilterPreAnalysis objects for a given query and + * also stores the per-query parameters that control the filter rewrite operations (from the query context). + * + * The analyses map is keyed by Filter: each Filter in the map belongs to separate level of query + * (e.g. outer query, subquery level 1, etc.) + * + * A concurrent hash map is used since the per-level Filter processing occurs across multiple threads. + */ public class JoinFilterPreAnalysisGroup { private final boolean enableFilterPushDown; diff --git a/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapterTest.java b/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapterTest.java index 3e77c5740d7c..df57afb614fe 100644 --- a/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapterTest.java @@ -2067,7 +2067,6 @@ public void test_makeCursors_factToCountryLeft_filterExcludesAllLeftRowsUsingLoo ); } - /* @Test public void test_makeCursors_originalFilterDoesNotMatchPreAnalysis_shouldThrowISE() { @@ -2081,24 +2080,27 @@ public void test_makeCursors_originalFilterDoesNotMatchPreAnalysis_shouldThrowIS ); Filter filter = new SelectorFilter("page", "this matches nothing"); + new HashJoinSegmentStorageAdapter( + factSegment.asStorageAdapter(), + joinableClauses, + joinFilterPreAnalysisGroup + ).makeCursors( + filter, + Intervals.ETERNITY, + VirtualColumns.EMPTY, + Granularities.ALL, + false, + null + ); + + /* try { - new HashJoinSegmentStorageAdapter( - factSegment.asStorageAdapter(), - joinableClauses, - joinFilterPreAnalysisGroup - ).makeCursors( - filter, - Intervals.ETERNITY, - VirtualColumns.EMPTY, - Granularities.ALL, - false, - null - ); + Assert.fail(); } catch (ISE e) { Assert.assertTrue(e.getMessage().startsWith("Filter provided to cursor [")); } + */ } - */ } diff --git a/processing/src/test/java/org/apache/druid/segment/join/JoinablesTest.java b/processing/src/test/java/org/apache/druid/segment/join/JoinablesTest.java index 4fa521dbcbc9..2f0b59550be6 100644 --- a/processing/src/test/java/org/apache/druid/segment/join/JoinablesTest.java +++ b/processing/src/test/java/org/apache/druid/segment/join/JoinablesTest.java @@ -28,7 +28,6 @@ import org.apache.druid.query.extraction.MapLookupExtractor; import org.apache.druid.query.planning.PreJoinableClause; import org.apache.druid.segment.Segment; -import org.apache.druid.segment.VirtualColumns; import org.apache.druid.segment.column.ColumnHolder; import org.apache.druid.segment.join.lookup.LookupJoinable; import org.junit.Assert; @@ -102,9 +101,7 @@ public void test_createSegmentMapFn_noClauses() QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_PUSH_DOWN, QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE, QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_VALUE_COLUMN_FILTERS, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE, - null, - VirtualColumns.EMPTY + QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE ); Assert.assertSame(Function.identity(), segmentMapFn); @@ -131,9 +128,7 @@ public void test_createSegmentMapFn_unusableClause() QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_PUSH_DOWN, QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE, QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_VALUE_COLUMN_FILTERS, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE, - null, - VirtualColumns.EMPTY + QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE ); } @@ -168,9 +163,7 @@ public void test_createSegmentMapFn_usableClause() QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_PUSH_DOWN, QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE, QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_VALUE_COLUMN_FILTERS, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE, - null, - VirtualColumns.EMPTY + QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE ); Assert.assertNotSame(Function.identity(), segmentMapFn); diff --git a/server/src/main/java/org/apache/druid/segment/realtime/appenderator/SinkQuerySegmentWalker.java b/server/src/main/java/org/apache/druid/segment/realtime/appenderator/SinkQuerySegmentWalker.java index 504daf3d60a0..5f0859863d2d 100644 --- a/server/src/main/java/org/apache/druid/segment/realtime/appenderator/SinkQuerySegmentWalker.java +++ b/server/src/main/java/org/apache/druid/segment/realtime/appenderator/SinkQuerySegmentWalker.java @@ -178,9 +178,7 @@ public QueryRunner getQueryRunnerForSegments(final Query query, final QueryContexts.getEnableJoinFilterPushDown(query), QueryContexts.getEnableJoinFilterRewrite(query), QueryContexts.getEnableJoinFilterRewriteValueColumnFilters(query), - QueryContexts.getJoinFilterRewriteMaxSize(query), - query.getFilter() == null ? null : query.getFilter().toFilter(), - query.getVirtualColumns() + QueryContexts.getJoinFilterRewriteMaxSize(query) ); Iterable> perSegmentRunners = Iterables.transform( diff --git a/server/src/main/java/org/apache/druid/server/LocalQuerySegmentWalker.java b/server/src/main/java/org/apache/druid/server/LocalQuerySegmentWalker.java index d7f39adaa4ba..ee453d9e07c5 100644 --- a/server/src/main/java/org/apache/druid/server/LocalQuerySegmentWalker.java +++ b/server/src/main/java/org/apache/druid/server/LocalQuerySegmentWalker.java @@ -100,9 +100,7 @@ public QueryRunner getQueryRunnerForIntervals(final Query query, final QueryContexts.getEnableJoinFilterPushDown(prioritizedAndLaned), QueryContexts.getEnableJoinFilterRewrite(prioritizedAndLaned), QueryContexts.getEnableJoinFilterRewriteValueColumnFilters(prioritizedAndLaned), - QueryContexts.getJoinFilterRewriteMaxSize(prioritizedAndLaned), - prioritizedAndLaned.getFilter() == null ? null : prioritizedAndLaned.getFilter().toFilter(), - prioritizedAndLaned.getVirtualColumns() + QueryContexts.getJoinFilterRewriteMaxSize(prioritizedAndLaned) ); final QueryRunnerFactory> queryRunnerFactory = conglomerate.findFactory(prioritizedAndLaned); diff --git a/server/src/main/java/org/apache/druid/server/coordination/ServerManager.java b/server/src/main/java/org/apache/druid/server/coordination/ServerManager.java index ca26e0dd4f75..624b02093f0c 100644 --- a/server/src/main/java/org/apache/druid/server/coordination/ServerManager.java +++ b/server/src/main/java/org/apache/druid/server/coordination/ServerManager.java @@ -201,9 +201,7 @@ public QueryRunner getQueryRunnerForSegments(Query query, Iterable> queryRunners = FunctionalIterable diff --git a/server/src/test/java/org/apache/druid/server/TestClusterQuerySegmentWalker.java b/server/src/test/java/org/apache/druid/server/TestClusterQuerySegmentWalker.java index cc3a406cfe78..e23780ae314f 100644 --- a/server/src/test/java/org/apache/druid/server/TestClusterQuerySegmentWalker.java +++ b/server/src/test/java/org/apache/druid/server/TestClusterQuerySegmentWalker.java @@ -146,9 +146,7 @@ public QueryRunner getQueryRunnerForSegments(final Query query, final QueryContexts.getEnableJoinFilterPushDown(query), QueryContexts.getEnableJoinFilterRewrite(query), QueryContexts.getEnableJoinFilterRewriteValueColumnFilters(query), - QueryContexts.getJoinFilterRewriteMaxSize(query), - query.getFilter() == null ? null : query.getFilter().toFilter(), - query.getVirtualColumns() + QueryContexts.getJoinFilterRewriteMaxSize(query) ); final QueryRunner baseRunner = new FinalizeResultsQueryRunner<>( From da4af6bd7a742173c74416a91ab94896349c49ea Mon Sep 17 00:00:00 2001 From: jon-wei Date: Wed, 3 Jun 2020 13:43:42 -0700 Subject: [PATCH 3/9] Remove unused imports and method --- processing/src/main/java/org/apache/druid/query/Query.java | 6 ------ .../main/java/org/apache/druid/segment/join/Joinables.java | 2 -- 2 files changed, 8 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/query/Query.java b/processing/src/main/java/org/apache/druid/query/Query.java index 03e26fe92a9d..56b837760cf1 100644 --- a/processing/src/main/java/org/apache/druid/query/Query.java +++ b/processing/src/main/java/org/apache/druid/query/Query.java @@ -37,7 +37,6 @@ import org.apache.druid.query.timeseries.TimeseriesQuery; import org.apache.druid.query.topn.TopNQuery; import org.apache.druid.segment.Segment; -import org.apache.druid.segment.VirtualColumns; import org.joda.time.DateTimeZone; import org.joda.time.Duration; import org.joda.time.Interval; @@ -170,9 +169,4 @@ default Query withLane(String lane) { return withOverriddenContext(ImmutableMap.of(QueryContexts.LANE_KEY, lane)); } - - default VirtualColumns getVirtualColumns() - { - return VirtualColumns.EMPTY; - } } diff --git a/processing/src/main/java/org/apache/druid/segment/join/Joinables.java b/processing/src/main/java/org/apache/druid/segment/join/Joinables.java index 3feee09ab4c5..3a8896f3990a 100644 --- a/processing/src/main/java/org/apache/druid/segment/join/Joinables.java +++ b/processing/src/main/java/org/apache/druid/segment/join/Joinables.java @@ -20,10 +20,8 @@ package org.apache.druid.segment.join; import org.apache.druid.java.util.common.IAE; -import org.apache.druid.query.filter.Filter; import org.apache.druid.query.planning.PreJoinableClause; import org.apache.druid.segment.Segment; -import org.apache.druid.segment.VirtualColumns; import org.apache.druid.segment.column.ColumnHolder; import org.apache.druid.segment.join.filter.JoinableClauses; import org.apache.druid.segment.join.filter.rewrite.JoinFilterPreAnalysisGroup; From f96752fcf4761f23a010931489c16fb9414d36da Mon Sep 17 00:00:00 2001 From: jon-wei Date: Wed, 3 Jun 2020 14:15:22 -0700 Subject: [PATCH 4/9] Fix overrides --- .../main/java/org/apache/druid/query/groupby/GroupByQuery.java | 1 - .../src/main/java/org/apache/druid/query/scan/ScanQuery.java | 1 - .../java/org/apache/druid/query/timeseries/TimeseriesQuery.java | 1 - .../src/main/java/org/apache/druid/query/topn/TopNQuery.java | 1 - 4 files changed, 4 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/query/groupby/GroupByQuery.java b/processing/src/main/java/org/apache/druid/query/groupby/GroupByQuery.java index 7bd2a847c081..4c17f16b03af 100644 --- a/processing/src/main/java/org/apache/druid/query/groupby/GroupByQuery.java +++ b/processing/src/main/java/org/apache/druid/query/groupby/GroupByQuery.java @@ -249,7 +249,6 @@ private List> verifySubtotalsSpec( return subtotalsSpec; } - @Override @JsonProperty public VirtualColumns getVirtualColumns() { 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 459b8326d217..719f5f27e6f7 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 @@ -183,7 +183,6 @@ private Integer validateAndGetMaxSegmentPartitionsOrderedInMemory() return maxSegmentPartitionsOrderedInMemory; } - @Override @JsonProperty public VirtualColumns getVirtualColumns() { diff --git a/processing/src/main/java/org/apache/druid/query/timeseries/TimeseriesQuery.java b/processing/src/main/java/org/apache/druid/query/timeseries/TimeseriesQuery.java index 729d839d0298..a959737cf645 100644 --- a/processing/src/main/java/org/apache/druid/query/timeseries/TimeseriesQuery.java +++ b/processing/src/main/java/org/apache/druid/query/timeseries/TimeseriesQuery.java @@ -103,7 +103,6 @@ public String getType() return Query.TIMESERIES; } - @Override @JsonProperty public VirtualColumns getVirtualColumns() { 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 3c301e584af8..c77011c0325d 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 @@ -113,7 +113,6 @@ public String getType() return TOPN; } - @Override @JsonProperty public VirtualColumns getVirtualColumns() { From 1726531d0ab70216a536dcaa4572257524153c61 Mon Sep 17 00:00:00 2001 From: jon-wei Date: Fri, 5 Jun 2020 18:52:15 -0700 Subject: [PATCH 5/9] Address PR comments --- .../java/org/apache/druid/query/Query.java | 6 + .../org/apache/druid/query/QueryContexts.java | 22 ++ .../druid/query/filter/AndDimFilter.java | 12 +- .../druid/query/filter/InDimFilter.java | 40 +- .../druid/query/filter/OrDimFilter.java | 12 +- .../druid/segment/filter/AndFilter.java | 8 +- .../apache/druid/segment/filter/InFilter.java | 27 +- .../apache/druid/segment/filter/OrFilter.java | 8 +- .../join/HashJoinSegmentStorageAdapter.java | 30 +- .../apache/druid/segment/join/Joinables.java | 48 +-- .../join/filter/JoinFilterAnalyzer.java | 28 +- .../join/filter/JoinFilterPreAnalysis.java | 26 +- .../rewrite/JoinFilterPreAnalysisGroup.java | 154 ++++++-- .../rewrite/JoinFilterRewriteConfig.java | 105 +++++ ...BaseHashJoinSegmentStorageAdapterTest.java | 23 +- .../HashJoinSegmentStorageAdapterTest.java | 261 ++----------- .../segment/join/HashJoinSegmentTest.java | 19 +- .../segment/join/JoinFilterAnalyzerTest.java | 367 ++++++++++++++---- .../druid/segment/join/JoinablesTest.java | 30 +- .../appenderator/SinkQuerySegmentWalker.java | 16 +- .../druid/server/LocalQuerySegmentWalker.java | 15 +- .../server/coordination/ServerManager.java | 16 +- .../server/TestClusterQuerySegmentWalker.java | 16 +- 23 files changed, 811 insertions(+), 478 deletions(-) create mode 100644 processing/src/main/java/org/apache/druid/segment/join/filter/rewrite/JoinFilterRewriteConfig.java diff --git a/processing/src/main/java/org/apache/druid/query/Query.java b/processing/src/main/java/org/apache/druid/query/Query.java index 56b837760cf1..03e26fe92a9d 100644 --- a/processing/src/main/java/org/apache/druid/query/Query.java +++ b/processing/src/main/java/org/apache/druid/query/Query.java @@ -37,6 +37,7 @@ import org.apache.druid.query.timeseries.TimeseriesQuery; import org.apache.druid.query.topn.TopNQuery; import org.apache.druid.segment.Segment; +import org.apache.druid.segment.VirtualColumns; import org.joda.time.DateTimeZone; import org.joda.time.Duration; import org.joda.time.Interval; @@ -169,4 +170,9 @@ default Query withLane(String lane) { return withOverriddenContext(ImmutableMap.of(QueryContexts.LANE_KEY, lane)); } + + default VirtualColumns getVirtualColumns() + { + return VirtualColumns.EMPTY; + } } diff --git a/processing/src/main/java/org/apache/druid/query/QueryContexts.java b/processing/src/main/java/org/apache/druid/query/QueryContexts.java index b5a7be0b7dd8..1c35004063e0 100644 --- a/processing/src/main/java/org/apache/druid/query/QueryContexts.java +++ b/processing/src/main/java/org/apache/druid/query/QueryContexts.java @@ -52,6 +52,7 @@ public class QueryContexts public static final String JOIN_FILTER_REWRITE_ENABLE_KEY = "enableJoinFilterRewrite"; public static final String JOIN_FILTER_REWRITE_VALUE_COLUMN_FILTERS_ENABLE_KEY = "enableJoinFilterRewriteValueColumnFilters"; public static final String JOIN_FILTER_REWRITE_MAX_SIZE_KEY = "joinFilterRewriteMaxSize"; + public static final String JOIN_FILTER_REWRITE_USE_OLD_REWRITE_MODE = "joinFilterRewriteUseOldRewriteMode"; public static final String USE_FILTER_CNF_KEY = "useFilterCNF"; public static final boolean DEFAULT_BY_SEGMENT = false; @@ -68,6 +69,7 @@ public class QueryContexts public static final boolean DEFAULT_ENABLE_JOIN_FILTER_PUSH_DOWN = true; public static final boolean DEFAULT_ENABLE_JOIN_FILTER_REWRITE = true; public static final boolean DEFAULT_ENABLE_JOIN_FILTER_REWRITE_VALUE_COLUMN_FILTERS = false; + public static final boolean DEFAULT_JOIN_FILTER_REWRITE_USE_OLD_REWRITE_MODE = false; public static final long DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE = 10000; public static final boolean DEFAULT_USE_FILTER_CNF = false; @@ -264,6 +266,26 @@ public static boolean getEnableJoinFilterRewrite(Query query) return parseBoolean(query, JOIN_FILTER_REWRITE_ENABLE_KEY, DEFAULT_ENABLE_JOIN_FILTER_REWRITE); } + /** + * This is an undocumented option provided as a transition tool: + * + * The join filter rewrites originally performed the pre-analysis phase prior to any per-segment processing, + * analyzing only the filter in the top-level of the query. + * + * This did not work for nested queries (see https://github.com/apache/druid/pull/9978), so the rewrite pre-analysis + * was moved into the cursor creation of the {@link org.apache.druid.segment.join.HashJoinSegmentStorageAdapter}. + * This design requires synchronization across multiple segment processing threads; the old rewrite mode + * is kept temporarily available in case issues arise with the new mode, and the user does not run queries with the + * affected nested shape. + */ + public static boolean getUseJoinFilterRewriteOldRewriteMode(Query query) + { + return parseBoolean( + query, + JOIN_FILTER_REWRITE_USE_OLD_REWRITE_MODE, + DEFAULT_JOIN_FILTER_REWRITE_USE_OLD_REWRITE_MODE + ); + } public static Query withMaxScatterGatherBytes(Query query, long maxScatterGatherBytesLimit) { 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 18df9d67a170..19e13db0fe29 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 @@ -20,12 +20,14 @@ package org.apache.druid.query.filter; import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Joiner; import com.google.common.base.Preconditions; import com.google.common.collect.RangeSet; import com.google.common.collect.TreeRangeSet; import org.apache.druid.java.util.common.StringUtils; +import org.apache.druid.query.cache.CacheKeyBuilder; import org.apache.druid.segment.filter.AndFilter; import org.apache.druid.segment.filter.Filters; @@ -43,6 +45,9 @@ public class AndDimFilter implements DimFilter private final List fields; + @JsonIgnore + private Integer fieldsHashCode; + @JsonCreator public AndDimFilter( @JsonProperty("fields") List fields @@ -67,7 +72,7 @@ public List getFields() @Override public byte[] getCacheKey() { - return DimFilterUtils.computeCacheKey(DimFilterUtils.AND_CACHE_ID, fields); + return new CacheKeyBuilder(DimFilterUtils.AND_CACHE_ID).appendInt(hashCode()).build(); } @Override @@ -142,7 +147,10 @@ public boolean equals(Object o) @Override public int hashCode() { - return fields != null ? fields.hashCode() : 0; + if (fieldsHashCode == null) { + fieldsHashCode = fields != null ? fields.hashCode() : 0; + } + return fieldsHashCode; } @Override 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 d5ec5588ba38..ffa75991d75d 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 @@ -34,6 +34,7 @@ import com.google.common.collect.Range; import com.google.common.collect.RangeSet; import com.google.common.collect.TreeRangeSet; +import com.google.common.hash.HashCode; import com.google.common.hash.Hasher; import com.google.common.hash.Hashing; import it.unimi.dsi.fastutil.ints.IntArrayList; @@ -81,6 +82,9 @@ public class InDimFilter implements DimFilter @JsonIgnore private byte[] cacheKey; + @JsonIgnore + private HashCode valuesHashCode; + @JsonCreator public InDimFilter( @JsonProperty("dimension") String dimension, @@ -149,28 +153,36 @@ public FilterTuning getFilterTuning() @Override public byte[] getCacheKey() { + if (valuesHashCode == null) { + valuesHashCode = computeValuesHashCode(values); + } if (cacheKey == null) { - final List sortedValues = new ArrayList<>(values); - sortedValues.sort(Comparator.nullsFirst(Ordering.natural())); - final Hasher hasher = Hashing.sha256().newHasher(); - for (String v : sortedValues) { - if (v == null) { - hasher.putInt(0); - } else { - hasher.putString(v, StandardCharsets.UTF_8); - } - } cacheKey = new CacheKeyBuilder(DimFilterUtils.IN_CACHE_ID) .appendString(dimension) .appendByte(DimFilterUtils.STRING_SEPARATOR) .appendByteArray(extractionFn == null ? new byte[0] : extractionFn.getCacheKey()) .appendByte(DimFilterUtils.STRING_SEPARATOR) - .appendByteArray(hasher.hash().asBytes()) + .appendByteArray(valuesHashCode.asBytes()) .build(); } return cacheKey; } + private static HashCode computeValuesHashCode(Set values) + { + final List sortedValues = new ArrayList<>(values); + sortedValues.sort(Comparator.nullsFirst(Ordering.natural())); + final Hasher hasher = Hashing.sha256().newHasher(); + for (String v : sortedValues) { + if (v == null) { + hasher.putInt(0); + } else { + hasher.putString(v, StandardCharsets.UTF_8); + } + } + return hasher.hash(); + } + @Override public DimFilter optimize() { @@ -237,7 +249,8 @@ public Filter toFilter() floatPredicateSupplier, doublePredicateSupplier, extractionFn, - filterTuning + filterTuning, + valuesHashCode ); } @@ -300,6 +313,9 @@ public boolean equals(Object o) @Override public int hashCode() { + if (valuesHashCode == null) { + valuesHashCode = computeValuesHashCode(values); + } return Objects.hash(values, dimension, extractionFn, filterTuning); } 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 ca8c105b7a3d..01d7e66fafc1 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 @@ -20,12 +20,14 @@ package org.apache.druid.query.filter; import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.Joiner; import com.google.common.base.Preconditions; import com.google.common.collect.RangeSet; import com.google.common.collect.TreeRangeSet; import org.apache.druid.java.util.common.StringUtils; +import org.apache.druid.query.cache.CacheKeyBuilder; import org.apache.druid.segment.filter.Filters; import org.apache.druid.segment.filter.OrFilter; @@ -44,6 +46,9 @@ public class OrDimFilter implements DimFilter private final List fields; + @JsonIgnore + private Integer fieldsHashCode; + @JsonCreator public OrDimFilter(@JsonProperty("fields") List fields) { @@ -75,7 +80,7 @@ public List getFields() @Override public byte[] getCacheKey() { - return DimFilterUtils.computeCacheKey(DimFilterUtils.OR_CACHE_ID, fields); + return new CacheKeyBuilder(DimFilterUtils.OR_CACHE_ID).appendInt(hashCode()).build(); } @Override @@ -148,7 +153,10 @@ public boolean equals(Object o) @Override public int hashCode() { - return fields != null ? fields.hashCode() : 0; + if (fieldsHashCode == null) { + fieldsHashCode = fields != null ? fields.hashCode() : 0; + } + return fieldsHashCode; } @Override diff --git a/processing/src/main/java/org/apache/druid/segment/filter/AndFilter.java b/processing/src/main/java/org/apache/druid/segment/filter/AndFilter.java index 696bd8b5c9c9..c0c05cc0aad8 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/AndFilter.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/AndFilter.java @@ -53,6 +53,8 @@ public class AndFilter implements BooleanFilter private final Set filters; + private Integer filtersHashCode; + @VisibleForTesting public AndFilter(List filters) { @@ -263,6 +265,10 @@ public boolean equals(Object o) @Override public int hashCode() { - return Objects.hash(getFilters()); + if (filtersHashCode == null) { + filtersHashCode = Objects.hash(getFilters()); + } + + return filtersHashCode; } } diff --git a/processing/src/main/java/org/apache/druid/segment/filter/InFilter.java b/processing/src/main/java/org/apache/druid/segment/filter/InFilter.java index dfc4d711d4ba..848730536656 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/InFilter.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/InFilter.java @@ -23,6 +23,7 @@ import com.google.common.base.Predicate; import com.google.common.base.Supplier; import com.google.common.collect.ImmutableSet; +import com.google.common.hash.HashCode; import it.unimi.dsi.fastutil.ints.IntIterable; import it.unimi.dsi.fastutil.ints.IntIterator; import org.apache.druid.collections.bitmap.ImmutableBitmap; @@ -72,6 +73,7 @@ public class InFilter implements Filter private final Supplier longPredicateSupplier; private final Supplier floatPredicateSupplier; private final Supplier doublePredicateSupplier; + private final HashCode valuesHashCode; public InFilter( String dimension, @@ -80,7 +82,8 @@ public InFilter( Supplier floatPredicateSupplier, Supplier doublePredicateSupplier, ExtractionFn extractionFn, - FilterTuning filterTuning + FilterTuning filterTuning, + HashCode valuesHashCode ) { this.dimension = dimension; @@ -90,6 +93,7 @@ public InFilter( this.longPredicateSupplier = longPredicateSupplier; this.floatPredicateSupplier = floatPredicateSupplier; this.doublePredicateSupplier = doublePredicateSupplier; + this.valuesHashCode = valuesHashCode; } @Override @@ -201,7 +205,8 @@ public Filter rewriteRequiredColumns(Map columnRewrites) floatPredicateSupplier, doublePredicateSupplier, extractionFn, - filterTuning + filterTuning, + valuesHashCode ); } @@ -225,7 +230,14 @@ public boolean supportsSelectivityEstimation(ColumnSelector columnSelector, Bitm private DruidPredicateFactory getPredicateFactory() { - return new InFilterDruidPredicateFactory(extractionFn, values, longPredicateSupplier, floatPredicateSupplier, doublePredicateSupplier); + return new InFilterDruidPredicateFactory( + extractionFn, + values, + longPredicateSupplier, + floatPredicateSupplier, + doublePredicateSupplier, + valuesHashCode + ); } @Override @@ -247,7 +259,7 @@ public boolean equals(Object o) @Override public int hashCode() { - return Objects.hash(dimension, values, extractionFn, filterTuning); + return Objects.hash(dimension, valuesHashCode, extractionFn, filterTuning); } @VisibleForTesting @@ -258,13 +270,15 @@ static class InFilterDruidPredicateFactory implements DruidPredicateFactory private final Supplier longPredicateSupplier; private final Supplier floatPredicateSupplier; private final Supplier doublePredicateSupplier; + private final HashCode valuesHashCode; InFilterDruidPredicateFactory( ExtractionFn extractionFn, Set values, Supplier longPredicateSupplier, Supplier floatPredicateSupplier, - Supplier doublePredicateSupplier + Supplier doublePredicateSupplier, + HashCode valuesHashCode ) { this.extractionFn = extractionFn; @@ -272,6 +286,7 @@ static class InFilterDruidPredicateFactory implements DruidPredicateFactory this.longPredicateSupplier = longPredicateSupplier; this.floatPredicateSupplier = floatPredicateSupplier; this.doublePredicateSupplier = doublePredicateSupplier; + this.valuesHashCode = valuesHashCode; } @Override @@ -330,7 +345,7 @@ public boolean equals(Object o) @Override public int hashCode() { - return Objects.hash(extractionFn, values); + return Objects.hash(extractionFn, valuesHashCode); } } } diff --git a/processing/src/main/java/org/apache/druid/segment/filter/OrFilter.java b/processing/src/main/java/org/apache/druid/segment/filter/OrFilter.java index 46cd9e246db4..2ba9a985d15d 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/OrFilter.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/OrFilter.java @@ -53,6 +53,8 @@ public class OrFilter implements BooleanFilter private final Set filters; + private Integer filtersHashCode; + @VisibleForTesting public OrFilter(List filters) { @@ -248,6 +250,10 @@ public boolean equals(Object o) @Override public int hashCode() { - return Objects.hash(getFilters()); + if (filtersHashCode == null) { + filtersHashCode = Objects.hash(getFilters()); + } + + return filtersHashCode; } } diff --git a/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapter.java b/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapter.java index 891932424a3e..7102bf64f448 100644 --- a/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapter.java +++ b/processing/src/main/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapter.java @@ -37,7 +37,6 @@ import org.apache.druid.segment.join.filter.JoinFilterAnalyzer; import org.apache.druid.segment.join.filter.JoinFilterPreAnalysis; import org.apache.druid.segment.join.filter.JoinFilterSplit; -import org.apache.druid.segment.join.filter.JoinableClauses; import org.apache.druid.segment.join.filter.rewrite.JoinFilterPreAnalysisGroup; import org.joda.time.DateTime; import org.joda.time.Interval; @@ -210,32 +209,13 @@ public Sequence makeCursors( ) { JoinFilterPreAnalysis jfpa; - if (filter == null) { - jfpa = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - joinFilterPreAnalysisGroup, - JoinableClauses.fromList(clauses), - virtualColumns, - null, - joinFilterPreAnalysisGroup.isEnableFilterPushDown(), - joinFilterPreAnalysisGroup.isEnableFilterRewrite(), - joinFilterPreAnalysisGroup.isEnableRewriteValueColumnFilters(), - joinFilterPreAnalysisGroup.getFilterRewriteMaxSize() - ); + if (joinFilterPreAnalysisGroup.getJoinFilterRewriteConfig().isOldRewriteMode()) { + jfpa = joinFilterPreAnalysisGroup.getPreAnalysisForOldRewriteMode(); } else { - jfpa = joinFilterPreAnalysisGroup.getAnalyses().computeIfAbsent( + jfpa = joinFilterPreAnalysisGroup.computeJoinFilterPreAnalysisIfAbsent( filter, - (theFilter) -> { - return JoinFilterAnalyzer.computeJoinFilterPreAnalysis( - joinFilterPreAnalysisGroup, - JoinableClauses.fromList(clauses), - virtualColumns, - theFilter, - joinFilterPreAnalysisGroup.isEnableFilterPushDown(), - joinFilterPreAnalysisGroup.isEnableFilterRewrite(), - joinFilterPreAnalysisGroup.isEnableRewriteValueColumnFilters(), - joinFilterPreAnalysisGroup.getFilterRewriteMaxSize() - ); - } + clauses, + virtualColumns ); } diff --git a/processing/src/main/java/org/apache/druid/segment/join/Joinables.java b/processing/src/main/java/org/apache/druid/segment/join/Joinables.java index 3a8896f3990a..c9c765484a1d 100644 --- a/processing/src/main/java/org/apache/druid/segment/join/Joinables.java +++ b/processing/src/main/java/org/apache/druid/segment/join/Joinables.java @@ -20,11 +20,14 @@ package org.apache.druid.segment.join; import org.apache.druid.java.util.common.IAE; +import org.apache.druid.query.filter.Filter; import org.apache.druid.query.planning.PreJoinableClause; import org.apache.druid.segment.Segment; +import org.apache.druid.segment.VirtualColumns; import org.apache.druid.segment.column.ColumnHolder; import org.apache.druid.segment.join.filter.JoinableClauses; import org.apache.druid.segment.join.filter.rewrite.JoinFilterPreAnalysisGroup; +import org.apache.druid.segment.join.filter.rewrite.JoinFilterRewriteConfig; import org.apache.druid.utils.JvmUtils; import javax.annotation.Nullable; @@ -68,28 +71,23 @@ public static boolean isPrefixedBy(final String columnName, final String prefix) /** * Creates a Function that maps base segments to {@link HashJoinSegment} if needed (i.e. if the number of join * clauses is > 0). If mapping is not needed, this method will return {@link Function#identity()}. - * @param clauses pre-joinable clauses - * @param joinableFactory factory for joinables - * @param cpuTimeAccumulator an accumulator that we will add CPU nanos to; this is part of the function to encourage - * callers to remember to track metrics on CPU time required for creation of Joinables - * @param enableFilterPushDown whether to enable filter push down optimizations to the base segment. In production -* this should generally be {@code QueryContexts.getEnableJoinFilterPushDown(query)}. - * @param enableFilterRewrite whether to enable filter rewrite optimizations for RHS columns. In production -* this should generally be {@code QueryContexts.getEnableJoinFilterRewrite(query)}. - * @param enableRewriteValueColumnFilters whether to enable filter rewrite optimizations for RHS columns that are not -* key columns. In production this should generally -* be {@code QueryContexts.getEnableJoinFilterRewriteValueColumnFilters(query)}. - * @param filterRewriteMaxSize the max allowed size of correlated value sets for RHS rewrites. In production -* this should generally be {@code QueryContexts.getJoinFilterRewriteMaxSize(query)}. + * @param clauses pre-joinable clauses + * @param joinableFactory factory for joinables + * @param cpuTimeAccumulator an accumulator that we will add CPU nanos to; this is part of the function to encourage + * callers to remember to track metrics on CPU time required for creation of Joinables + * @param joinFilterRewriteConfig Configuration options for the join filter rewrites + * @param originalTopLevelFilterForOldRewriteMode The filter from the top level of the query. Only used if + * joinFilterRewriteConfig.isOldRewriteMode() is true. + * @param virtualColumnsForOldRewriteMode The virtual columns from the top level of the query. Only used if + * joinFilterRewriteConfig.isOldRewriteMode() is true. */ public static Function createSegmentMapFn( final List clauses, final JoinableFactory joinableFactory, final AtomicLong cpuTimeAccumulator, - final boolean enableFilterPushDown, - final boolean enableFilterRewrite, - final boolean enableRewriteValueColumnFilters, - final long filterRewriteMaxSize + final JoinFilterRewriteConfig joinFilterRewriteConfig, + @Nullable final Filter originalTopLevelFilterForOldRewriteMode, + @Nullable final VirtualColumns virtualColumnsForOldRewriteMode ) { // compute column correlations here and RHS correlated values @@ -100,12 +98,16 @@ public static Function createSegmentMapFn( return Function.identity(); } else { final JoinableClauses joinableClauses = JoinableClauses.createClauses(clauses, joinableFactory); - final JoinFilterPreAnalysisGroup jfpag = new JoinFilterPreAnalysisGroup( - enableFilterPushDown, - enableFilterRewrite, - enableRewriteValueColumnFilters, - filterRewriteMaxSize - ); + final JoinFilterPreAnalysisGroup jfpag = new JoinFilterPreAnalysisGroup(joinFilterRewriteConfig); + + // compatiblity mode + if (joinFilterRewriteConfig.isOldRewriteMode()) { + jfpag.performAnalysisForOldRewriteMode( + originalTopLevelFilterForOldRewriteMode, + joinableClauses.getJoinableClauses(), + virtualColumnsForOldRewriteMode + ); + } return baseSegment -> new HashJoinSegment(baseSegment, joinableClauses.getJoinableClauses(), jfpag); } diff --git a/processing/src/main/java/org/apache/druid/segment/join/filter/JoinFilterAnalyzer.java b/processing/src/main/java/org/apache/druid/segment/join/filter/JoinFilterAnalyzer.java index 91929a450b6d..06475ad89e51 100644 --- a/processing/src/main/java/org/apache/druid/segment/join/filter/JoinFilterAnalyzer.java +++ b/processing/src/main/java/org/apache/druid/segment/join/filter/JoinFilterAnalyzer.java @@ -32,7 +32,7 @@ import org.apache.druid.segment.filter.Filters; import org.apache.druid.segment.filter.OrFilter; import org.apache.druid.segment.filter.SelectorFilter; -import org.apache.druid.segment.join.filter.rewrite.JoinFilterPreAnalysisGroup; +import org.apache.druid.segment.join.filter.rewrite.JoinFilterRewriteConfig; import org.apache.druid.segment.virtual.ExpressionVirtualColumn; import java.util.ArrayList; @@ -83,29 +83,17 @@ public class JoinFilterAnalyzer * * See {@link JoinFilterPreAnalysis} for details on the result of this pre-analysis step. * - * @param joinFilterPreAnalysisGroup The query-scoped pre analysis group, used to hold pre-analysis results - * on a per-filter basis * @param joinableClauses The joinable clauses from the query * @param virtualColumns The virtual columns from the query * @param originalFilter The original filter from the query - * @param enableFilterPushDown Whether to enable filter push down - * @param enableFilterRewrite Whether to enable rewrites of filters involving RHS columns - * @param enableRewriteValueColumnFilters Whether to enable rewrites of filters invovling RHS non-key columns - * @param filterRewriteMaxSize The maximum size of the correlated value set for rewritten filters. - * If the correlated value set size exceeds this, the filter will not be - * rewritten and pushed down. - * + * @param joinFilterRewriteConfig Configuration options for the join rewrites * @return A JoinFilterPreAnalysis containing information determined in this pre-analysis step. */ public static JoinFilterPreAnalysis computeJoinFilterPreAnalysis( - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup, JoinableClauses joinableClauses, VirtualColumns virtualColumns, Filter originalFilter, - boolean enableFilterPushDown, - boolean enableFilterRewrite, - boolean enableRewriteValueColumnFilters, - long filterRewriteMaxSize + JoinFilterRewriteConfig joinFilterRewriteConfig ) { final List preJoinVirtualColumns = new ArrayList<>(); @@ -113,8 +101,8 @@ public static JoinFilterPreAnalysis computeJoinFilterPreAnalysis( joinableClauses.splitVirtualColumns(virtualColumns, preJoinVirtualColumns, postJoinVirtualColumns); JoinFilterPreAnalysis.Builder preAnalysisBuilder = - new JoinFilterPreAnalysis.Builder(joinFilterPreAnalysisGroup, joinableClauses, originalFilter, postJoinVirtualColumns); - if (originalFilter == null || !enableFilterPushDown) { + new JoinFilterPreAnalysis.Builder(joinFilterRewriteConfig, joinableClauses, originalFilter, postJoinVirtualColumns); + if (originalFilter == null || !joinFilterRewriteConfig.isEnableFilterPushDown()) { return preAnalysisBuilder.build(); } @@ -137,7 +125,7 @@ public static JoinFilterPreAnalysis computeJoinFilterPreAnalysis( preAnalysisBuilder .withNormalizedBaseTableClauses(normalizedBaseTableClauses) .withNormalizedJoinTableClauses(normalizedJoinTableClauses); - if (!enableFilterRewrite) { + if (!joinFilterRewriteConfig.isEnableFilterRewrite()) { return preAnalysisBuilder.build(); } @@ -148,8 +136,8 @@ public static JoinFilterPreAnalysis computeJoinFilterPreAnalysis( normalizedJoinTableClauses, equiconditions, joinableClauses, - enableRewriteValueColumnFilters, - filterRewriteMaxSize + joinFilterRewriteConfig.isEnableRewriteValueColumnFilters(), + joinFilterRewriteConfig.getFilterRewriteMaxSize() ); return preAnalysisBuilder.withCorrelations(correlations) diff --git a/processing/src/main/java/org/apache/druid/segment/join/filter/JoinFilterPreAnalysis.java b/processing/src/main/java/org/apache/druid/segment/join/filter/JoinFilterPreAnalysis.java index da48f0536f26..de842c3dde02 100644 --- a/processing/src/main/java/org/apache/druid/segment/join/filter/JoinFilterPreAnalysis.java +++ b/processing/src/main/java/org/apache/druid/segment/join/filter/JoinFilterPreAnalysis.java @@ -24,7 +24,7 @@ import org.apache.druid.segment.VirtualColumn; import org.apache.druid.segment.join.Equality; import org.apache.druid.segment.join.JoinableClause; -import org.apache.druid.segment.join.filter.rewrite.JoinFilterPreAnalysisGroup; +import org.apache.druid.segment.join.filter.rewrite.JoinFilterRewriteConfig; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -43,7 +43,7 @@ * - A list of filter clauses from the original filter's CNF representation that only reference the base table * - A list of filter clauses from the original filter's CNF representation that reference RHS join tables * - A list of virtual columns that can only be computed post-join - * - The JoinFilterPreAnalysisGroup that this pre-analysis is associated with. + * - The JoinFilterRewriteConfig that this pre-analysis is associated with. */ public class JoinFilterPreAnalysis { @@ -52,9 +52,9 @@ public class JoinFilterPreAnalysis private final List normalizedBaseTableClauses; private final List normalizedJoinTableClauses; private final JoinFilterCorrelations correlations; - private final JoinFilterPreAnalysisGroup myGroup; private final List postJoinVirtualColumns; private final Equiconditions equiconditions; + private final JoinFilterRewriteConfig rewriteConfig; private JoinFilterPreAnalysis( final JoinableClauses joinableClauses, @@ -63,8 +63,8 @@ private JoinFilterPreAnalysis( final List normalizedBaseTableClauses, final List normalizedJoinTableClauses, JoinFilterCorrelations correlations, - final JoinFilterPreAnalysisGroup myGroup, - final Equiconditions equiconditions + final Equiconditions equiconditions, + final JoinFilterRewriteConfig rewriteConfig ) { this.joinableClauses = joinableClauses; @@ -73,7 +73,7 @@ private JoinFilterPreAnalysis( this.normalizedBaseTableClauses = normalizedBaseTableClauses; this.normalizedJoinTableClauses = normalizedJoinTableClauses; this.correlations = correlations; - this.myGroup = myGroup; + this.rewriteConfig = rewriteConfig; this.equiconditions = equiconditions; } @@ -114,12 +114,12 @@ public Map> getCorrelationsByD public boolean isEnableFilterPushDown() { - return myGroup.isEnableFilterPushDown(); + return rewriteConfig.isEnableFilterPushDown(); } public boolean isEnableFilterRewrite() { - return myGroup.isEnableFilterRewrite(); + return rewriteConfig.isEnableFilterRewrite(); } public Equiconditions getEquiconditions() @@ -132,7 +132,7 @@ public Equiconditions getEquiconditions() */ public static class Builder { - @Nonnull private final JoinFilterPreAnalysisGroup group; + @Nonnull private final JoinFilterRewriteConfig rewriteConfig; @Nonnull private final JoinableClauses joinableClauses; @Nullable private final Filter originalFilter; @Nullable private List normalizedBaseTableClauses; @@ -142,13 +142,13 @@ public static class Builder @Nonnull private Equiconditions equiconditions = new Equiconditions(Collections.emptyMap()); public Builder( - @Nonnull JoinFilterPreAnalysisGroup group, + @Nonnull JoinFilterRewriteConfig rewriteConfig, @Nonnull JoinableClauses joinableClauses, @Nullable Filter originalFilter, @Nonnull List postJoinVirtualColumns ) { - this.group = group; + this.rewriteConfig = rewriteConfig; this.joinableClauses = joinableClauses; this.originalFilter = originalFilter; this.postJoinVirtualColumns = postJoinVirtualColumns; @@ -199,8 +199,8 @@ public JoinFilterPreAnalysis build() normalizedBaseTableClauses, normalizedJoinTableClauses, correlations, - group, - equiconditions + equiconditions, + rewriteConfig ); } diff --git a/processing/src/main/java/org/apache/druid/segment/join/filter/rewrite/JoinFilterPreAnalysisGroup.java b/processing/src/main/java/org/apache/druid/segment/join/filter/rewrite/JoinFilterPreAnalysisGroup.java index 0b3af2f61de3..ae3f6fa228f1 100644 --- a/processing/src/main/java/org/apache/druid/segment/join/filter/rewrite/JoinFilterPreAnalysisGroup.java +++ b/processing/src/main/java/org/apache/druid/segment/join/filter/rewrite/JoinFilterPreAnalysisGroup.java @@ -20,63 +20,167 @@ package org.apache.druid.segment.join.filter.rewrite; import org.apache.druid.query.filter.Filter; +import org.apache.druid.segment.VirtualColumns; +import org.apache.druid.segment.join.JoinableClause; +import org.apache.druid.segment.join.filter.JoinFilterAnalyzer; import org.apache.druid.segment.join.filter.JoinFilterPreAnalysis; +import org.apache.druid.segment.join.filter.JoinableClauses; +import java.util.List; +import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; /** * A JoinFilterPreAnalysisGroup holds all of the JoinFilterPreAnalysis objects for a given query and * also stores the per-query parameters that control the filter rewrite operations (from the query context). * - * The analyses map is keyed by Filter: each Filter in the map belongs to separate level of query - * (e.g. outer query, subquery level 1, etc.) + * The analyses map is keyed by (Filter, JoinableClause list, VirtualColumns): each Filter in the map belongs to a + * separate level of query (e.g. outer query, subquery level 1, etc.) * * A concurrent hash map is used since the per-level Filter processing occurs across multiple threads. */ public class JoinFilterPreAnalysisGroup { - private final boolean enableFilterPushDown; - private final boolean enableFilterRewrite; - private final boolean enableRewriteValueColumnFilters; - private final long filterRewriteMaxSize; - private final ConcurrentHashMap analyses; + private final JoinFilterRewriteConfig joinFilterRewriteConfig; + private final ConcurrentHashMap analyses; + + /** + * This is an undocumented option provided as a transition tool: + * + * The join filter rewrites originally performed the pre-analysis phase prior to any per-segment processing, + * analyzing only the filter in the top-level of the query. + * + * This did not work for nested queries (see https://github.com/apache/druid/pull/9978), so the rewrite pre-analysis + * was moved into the cursor creation of the {@link org.apache.druid.segment.join.HashJoinSegmentStorageAdapter}. + * This design requires synchronization across multiple segment processing threads; the old rewrite mode + * is kept temporarily available in case issues arise with the new mode, and the user does not run queries with the + * affected nested shape. + */ + private JoinFilterPreAnalysis preAnalysisForOldRewriteMode; public JoinFilterPreAnalysisGroup( - boolean enableFilterPushDown, - boolean enableFilterRewrite, - boolean enableRewriteValueColumnFilters, - long filterRewriteMaxSize + JoinFilterRewriteConfig joinFilterRewriteConfig ) { - this.enableFilterPushDown = enableFilterPushDown; - this.enableFilterRewrite = enableFilterRewrite; - this.enableRewriteValueColumnFilters = enableRewriteValueColumnFilters; - this.filterRewriteMaxSize = filterRewriteMaxSize; + this.joinFilterRewriteConfig = joinFilterRewriteConfig; this.analyses = new ConcurrentHashMap<>(); } - public boolean isEnableFilterPushDown() + public JoinFilterRewriteConfig getJoinFilterRewriteConfig() { - return enableFilterPushDown; + return joinFilterRewriteConfig; } - public boolean isEnableFilterRewrite() + public JoinFilterPreAnalysis computeJoinFilterPreAnalysisIfAbsent( + Filter filter, + List clauses, + VirtualColumns virtualColumns + ) { - return enableFilterRewrite; + // Some filters have potentially expensive hash codes that are lazily computed and cached. + // We call hashCode() here in a synchronized block before we attempt to use the Filter in the analyses map, + // to ensure that the hashCode is only computed once per Filter since the Filter interface is not thread-safe. + synchronized (analyses) { + if (filter != null) { + filter.hashCode(); + } + } + + JoinFilterPreAnalysisGroupKey key = new JoinFilterPreAnalysisGroupKey(filter, clauses, virtualColumns); + + return analyses.computeIfAbsent( + key, + (groupKey) -> { + return JoinFilterAnalyzer.computeJoinFilterPreAnalysis( + JoinableClauses.fromList(clauses), + virtualColumns, + filter, + joinFilterRewriteConfig + ); + } + ); } - public boolean isEnableRewriteValueColumnFilters() + public JoinFilterPreAnalysis getAnalysis( + Filter filter, + List clauses, + VirtualColumns virtualColumns + ) { - return enableRewriteValueColumnFilters; + JoinFilterPreAnalysisGroupKey key = new JoinFilterPreAnalysisGroupKey(filter, clauses, virtualColumns); + return analyses.get(key); } - public long getFilterRewriteMaxSize() + public void performAnalysisForOldRewriteMode( + Filter filter, + List clauses, + VirtualColumns virtualColumns + ) { - return filterRewriteMaxSize; + preAnalysisForOldRewriteMode = JoinFilterAnalyzer.computeJoinFilterPreAnalysis( + JoinableClauses.fromList(clauses), + virtualColumns, + filter, + joinFilterRewriteConfig + ); } - public ConcurrentHashMap getAnalyses() + public JoinFilterPreAnalysis getPreAnalysisForOldRewriteMode() { - return analyses; + return preAnalysisForOldRewriteMode; + } + + private static class JoinFilterPreAnalysisGroupKey + { + private final Filter filter; + private final List clauses; + private final VirtualColumns virtualColumns; + + public JoinFilterPreAnalysisGroupKey( + Filter filter, + List clauses, + VirtualColumns virtualColumns + ) + { + this.filter = filter; + this.clauses = clauses; + this.virtualColumns = virtualColumns; + } + + public Filter getFilter() + { + return filter; + } + + public List getClauses() + { + return clauses; + } + + public VirtualColumns getVirtualColumns() + { + return virtualColumns; + } + + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + JoinFilterPreAnalysisGroupKey that = (JoinFilterPreAnalysisGroupKey) o; + return Objects.equals(filter, that.filter) && + Objects.equals(clauses, that.clauses) && + Objects.equals(virtualColumns, that.virtualColumns); + } + + @Override + public int hashCode() + { + return Objects.hash(filter, clauses, virtualColumns); + } } } diff --git a/processing/src/main/java/org/apache/druid/segment/join/filter/rewrite/JoinFilterRewriteConfig.java b/processing/src/main/java/org/apache/druid/segment/join/filter/rewrite/JoinFilterRewriteConfig.java new file mode 100644 index 000000000000..5f64d6a57ccf --- /dev/null +++ b/processing/src/main/java/org/apache/druid/segment/join/filter/rewrite/JoinFilterRewriteConfig.java @@ -0,0 +1,105 @@ +/* + * 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.segment.join.filter.rewrite; + +/** + * A config class that holds properties that control how join filter rewrites behave. + */ +public class JoinFilterRewriteConfig +{ + /** + * Whether to enable filter push down optimizations to the base segment. + * In production this should generally be {@code QueryContexts.getEnableJoinFilterPushDown(query)}. + */ + private final boolean enableFilterPushDown; + + /** + * Whether to enable filter rewrite optimizations for RHS columns. + * In production this should generally be {@code QueryContexts.getEnableJoinFilterRewrite(query)}. + */ + private final boolean enableFilterRewrite; + + /** + * Whether to enable filter rewrite optimizations for RHS columns that are not key columns. + * In production this should generally be {@code QueryContexts.getEnableJoinFilterRewriteValueColumnFilters(query)}. + */ + private final boolean enableRewriteValueColumnFilters; + + /** + * The max allowed size of correlated value sets for RHS rewrites. In production + * This should generally be {@code QueryContexts.getJoinFilterRewriteMaxSize(query)}. + */ + private final long filterRewriteMaxSize; + + /** + * This is an undocumented option provided as a transition tool: + * + * The join filter rewrites originally performed the pre-analysis phase prior to any per-segment processing, + * analyzing only the filter in the top-level of the query. + * + * This did not work for nested queries (see https://github.com/apache/druid/pull/9978), so the rewrite pre-analysis + * was moved into the cursor creation of the {@link org.apache.druid.segment.join.HashJoinSegmentStorageAdapter}. + * This design requires synchronization across multiple segment processing threads; the old rewrite mode + * is kept temporarily available in case issues arise with the new mode, and the user does not run queries with the + * affected nested shape. + */ + private final boolean oldRewriteMode; + + + public JoinFilterRewriteConfig( + boolean enableFilterPushDown, + boolean enableFilterRewrite, + boolean enableRewriteValueColumnFilters, + long filterRewriteMaxSize, + boolean oldRewriteMode + ) + { + this.enableFilterPushDown = enableFilterPushDown; + this.enableFilterRewrite = enableFilterRewrite; + this.enableRewriteValueColumnFilters = enableRewriteValueColumnFilters; + this.filterRewriteMaxSize = filterRewriteMaxSize; + this.oldRewriteMode = oldRewriteMode; + } + + public boolean isEnableFilterPushDown() + { + return enableFilterPushDown; + } + + public boolean isEnableFilterRewrite() + { + return enableFilterRewrite; + } + + public boolean isEnableRewriteValueColumnFilters() + { + return enableRewriteValueColumnFilters; + } + + public long getFilterRewriteMaxSize() + { + return filterRewriteMaxSize; + } + + public boolean isOldRewriteMode() + { + return oldRewriteMode; + } +} diff --git a/processing/src/test/java/org/apache/druid/segment/join/BaseHashJoinSegmentStorageAdapterTest.java b/processing/src/test/java/org/apache/druid/segment/join/BaseHashJoinSegmentStorageAdapterTest.java index 16e46ec22bb5..402047576bf0 100644 --- a/processing/src/test/java/org/apache/druid/segment/join/BaseHashJoinSegmentStorageAdapterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/join/BaseHashJoinSegmentStorageAdapterTest.java @@ -27,6 +27,7 @@ import org.apache.druid.query.lookup.LookupExtractor; import org.apache.druid.segment.QueryableIndexSegment; import org.apache.druid.segment.join.filter.rewrite.JoinFilterPreAnalysisGroup; +import org.apache.druid.segment.join.filter.rewrite.JoinFilterRewriteConfig; import org.apache.druid.segment.join.lookup.LookupJoinable; import org.apache.druid.segment.join.table.IndexedTable; import org.apache.druid.segment.join.table.IndexedTableJoinable; @@ -44,6 +45,14 @@ public class BaseHashJoinSegmentStorageAdapterTest { + public static JoinFilterRewriteConfig DEFAULT_JOIN_FILTER_REWRITE_CONFIG = new JoinFilterRewriteConfig( + true, + true, + true, + QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE, + false + ); + public static final String FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX = "c1."; public static final String FACT_TO_COUNTRY_ON_NUMBER_PREFIX = "c2."; public static final String FACT_TO_REGION_PREFIX = "r1."; @@ -184,12 +193,7 @@ protected JoinableClause regionToCountry(final JoinType joinType) protected HashJoinSegmentStorageAdapter makeFactToCountrySegment() { - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); return new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), @@ -216,4 +220,11 @@ protected void compareExpressionVirtualColumns( actualVirtualColumn.getParsedExpression().get().toString() ); } + + protected static JoinFilterPreAnalysisGroup makeDefaultConfigPreAnalysisGroup() + { + return new JoinFilterPreAnalysisGroup( + DEFAULT_JOIN_FILTER_REWRITE_CONFIG + ); + } } diff --git a/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapterTest.java b/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapterTest.java index df57afb614fe..d65dbce7bf37 100644 --- a/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentStorageAdapterTest.java @@ -27,7 +27,6 @@ import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.java.util.common.granularity.Granularities; import org.apache.druid.math.expr.ExprMacroTable; -import org.apache.druid.query.QueryContexts; import org.apache.druid.query.filter.ExpressionDimFilter; import org.apache.druid.query.filter.Filter; import org.apache.druid.query.filter.OrDimFilter; @@ -299,12 +298,7 @@ public void test_makeCursors_factToCountryLeft() { List joinableClauses = ImmutableList.of(factToCountryOnIsoCode(JoinType.LEFT)); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.verifyCursors( new HashJoinSegmentStorageAdapter( @@ -365,12 +359,7 @@ public void test_makeCursors_factToCountryLeftUsingLookup() { List joinableClauses = ImmutableList.of(factToCountryNameUsingIsoCodeLookup(JoinType.LEFT)); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.verifyCursors( new HashJoinSegmentStorageAdapter( @@ -429,12 +418,7 @@ public void test_makeCursors_factToCountryLeftUsingLookup() public void test_makeCursors_factToCountryInner() { List joinableClauses = ImmutableList.of(factToCountryOnIsoCode(JoinType.INNER)); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.verifyCursors( new HashJoinSegmentStorageAdapter( @@ -488,12 +472,7 @@ public void test_makeCursors_factToCountryInner() public void test_makeCursors_factToCountryInnerUsingLookup() { List joinableClauses = ImmutableList.of(factToCountryNameUsingIsoCodeLookup(JoinType.INNER)); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.verifyCursors( new HashJoinSegmentStorageAdapter( @@ -549,12 +528,7 @@ public void test_makeCursors_factToCountryInnerUsingCountryNumber() // is interpreted as 0 (a.k.a. Australia). List joinableClauses = ImmutableList.of(factToCountryOnNumber(JoinType.INNER)); Filter filter = new SelectorDimFilter("channel", "#en.wikipedia", null).toFilter(); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.verifyCursors( new HashJoinSegmentStorageAdapter( @@ -616,12 +590,7 @@ public void test_makeCursors_factToCountryInnerUsingCountryNumberUsingLookup() // is interpreted as 0 (a.k.a. Australia). List joinableClauses = ImmutableList.of(factToCountryNameUsingNumberLookup(JoinType.INNER)); Filter filter = new SelectorDimFilter("channel", "#en.wikipedia", null).toFilter(); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.verifyCursors( new HashJoinSegmentStorageAdapter( @@ -679,12 +648,7 @@ public void test_makeCursors_factToCountryLeftWithFilterOnFacts() { List joinableClauses = ImmutableList.of(factToCountryOnIsoCode(JoinType.LEFT)); Filter filter = new SelectorDimFilter("channel", "#de.wikipedia", null).toFilter(); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.verifyCursors( new HashJoinSegmentStorageAdapter( @@ -717,12 +681,7 @@ public void test_makeCursors_factToCountryLeftWithFilterOnFactsUsingLookup() { List joinableClauses = ImmutableList.of(factToCountryNameUsingIsoCodeLookup(JoinType.LEFT)); Filter filter = new SelectorDimFilter("channel", "#de.wikipedia", null).toFilter(); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.verifyCursors( new HashJoinSegmentStorageAdapter( @@ -754,12 +713,7 @@ public void test_makeCursors_factToCountryRightWithFilterOnLeftIsNull() { List joinableClauses = ImmutableList.of(factToCountryOnIsoCode(JoinType.RIGHT)); Filter filter = new SelectorDimFilter("channel", null, null).toFilter(); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.verifyCursors( new HashJoinSegmentStorageAdapter( @@ -794,12 +748,7 @@ public void test_makeCursors_factToCountryRightWithFilterOnLeftIsNullUsingLookup { List joinableClauses = ImmutableList.of(factToCountryNameUsingIsoCodeLookup(JoinType.RIGHT)); Filter filter = new SelectorDimFilter("channel", null, null).toFilter(); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.verifyCursors( new HashJoinSegmentStorageAdapter( @@ -833,12 +782,7 @@ public void test_makeCursors_factToCountryFullWithFilterOnLeftIsNull() { List joinableClauses = ImmutableList.of(factToCountryOnIsoCode(JoinType.FULL)); Filter filter = new SelectorDimFilter("channel", null, null).toFilter(); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.verifyCursors( new HashJoinSegmentStorageAdapter( @@ -873,12 +817,7 @@ public void test_makeCursors_factToCountryFullWithFilterOnLeftIsNullUsingLookup( { List joinableClauses = ImmutableList.of(factToCountryNameUsingIsoCodeLookup(JoinType.FULL)); Filter filter = new SelectorDimFilter("channel", null, null).toFilter(); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.verifyCursors( new HashJoinSegmentStorageAdapter( @@ -917,12 +856,7 @@ public void test_makeCursors_factToCountryRightWithFilterOnJoinable() null ).toFilter(); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.verifyCursors( new HashJoinSegmentStorageAdapter( @@ -961,12 +895,7 @@ public void test_makeCursors_factToCountryRightWithFilterOnJoinableUsingLookup() null ).toFilter(); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.verifyCursors( new HashJoinSegmentStorageAdapter( @@ -1005,12 +934,7 @@ public void test_makeCursors_factToCountryLeftWithFilterOnJoinable() new SelectorDimFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "countryNumber", "10", null) ).toFilter(); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.verifyCursors( new HashJoinSegmentStorageAdapter( @@ -1049,12 +973,7 @@ public void test_makeCursors_factToCountryLeftWithFilterOnJoinableUsingLookup() new SelectorDimFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "v", "Norway", null) ).toFilter(); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.verifyCursors( new HashJoinSegmentStorageAdapter( @@ -1106,12 +1025,7 @@ public void test_makeCursors_factToCountryInnerWithFilterInsteadOfRealJoinCondit ExprMacroTable.nil() ).toFilter(); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.verifyCursors( new HashJoinSegmentStorageAdapter( @@ -1184,13 +1098,7 @@ public void test_makeCursors_factToCountryInnerWithFilterInsteadOfRealJoinCondit StringUtils.format("\"%sk\" == countryIsoCode", FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX), ExprMacroTable.nil() ).toFilter(); - - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.verifyCursors( new HashJoinSegmentStorageAdapter( @@ -1246,13 +1154,7 @@ public void test_makeCursors_factToRegionToCountryLeft() factToRegion(JoinType.LEFT), regionToCountry(JoinType.LEFT) ); - - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.verifyCursors( new HashJoinSegmentStorageAdapter( @@ -1323,12 +1225,8 @@ public void test_makeCursors_factToCountryAlwaysTrue() ); Filter filter = new SelectorDimFilter("channel", "#de.wikipedia", null).toFilter(); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); + JoinTestHelper.verifyCursors( new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), @@ -1387,12 +1285,7 @@ public void test_makeCursors_factToCountryAlwaysFalse() Filter filter = new SelectorDimFilter("channel", "#de.wikipedia", null).toFilter(); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.verifyCursors( new HashJoinSegmentStorageAdapter( @@ -1435,12 +1328,7 @@ public void test_makeCursors_factToCountryAlwaysTrueUsingLookup() Filter filter = new SelectorDimFilter("channel", "#de.wikipedia", null).toFilter(); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.verifyCursors( new HashJoinSegmentStorageAdapter( @@ -1500,12 +1388,7 @@ public void test_makeCursors_factToCountryAlwaysFalseUsingLookup() Filter filter = new SelectorDimFilter("channel", "#de.wikipedia", null).toFilter(); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.verifyCursors( new HashJoinSegmentStorageAdapter( @@ -1557,12 +1440,7 @@ public void test_makeCursors_factToCountryUsingVirtualColumn() ) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.verifyCursors( new HashJoinSegmentStorageAdapter( @@ -1620,12 +1498,7 @@ public void test_makeCursors_factToCountryUsingVirtualColumnUsingLookup() ) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.verifyCursors( new HashJoinSegmentStorageAdapter( @@ -1675,12 +1548,7 @@ public void test_makeCursors_factToCountryUsingExpression() ) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.verifyCursors( new HashJoinSegmentStorageAdapter( @@ -1729,12 +1597,7 @@ public void test_makeCursors_factToCountryUsingExpressionUsingLookup() ) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.verifyCursors( new HashJoinSegmentStorageAdapter( @@ -1785,13 +1648,7 @@ public void test_makeCursors_factToRegionTheWrongWay() ); Filter filter = new SelectorDimFilter("regionIsoCode", "VA", null).toFilter(); - - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.verifyCursors( new HashJoinSegmentStorageAdapter( @@ -1843,12 +1700,7 @@ public void test_makeCursors_errorOnNonEquiJoin() ) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.readCursors( new HashJoinSegmentStorageAdapter( @@ -1886,12 +1738,7 @@ public void test_makeCursors_errorOnNonEquiJoinUsingLookup() ) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.readCursors( new HashJoinSegmentStorageAdapter( @@ -1929,12 +1776,7 @@ public void test_makeCursors_errorOnNonKeyBasedJoin() ) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.readCursors( new HashJoinSegmentStorageAdapter( @@ -1971,12 +1813,7 @@ public void test_makeCursors_errorOnNonKeyBasedJoinUsingLookup() ) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.readCursors( new HashJoinSegmentStorageAdapter( @@ -2001,12 +1838,7 @@ public void test_makeCursors_factToCountryLeft_filterExcludesAllLeftRows() Filter originalFilter = new SelectorFilter("page", "this matches nothing"); List joinableClauses = ImmutableList.of(factToCountryOnIsoCode(JoinType.LEFT)); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.verifyCursors( new HashJoinSegmentStorageAdapter( @@ -2037,12 +1869,7 @@ public void test_makeCursors_factToCountryLeft_filterExcludesAllLeftRowsUsingLoo { Filter originalFilter = new SelectorFilter("page", "this matches nothing"); List joinableClauses = ImmutableList.of(factToCountryNameUsingIsoCodeLookup(JoinType.LEFT)); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); JoinTestHelper.verifyCursors( new HashJoinSegmentStorageAdapter( @@ -2072,12 +1899,7 @@ public void test_makeCursors_originalFilterDoesNotMatchPreAnalysis_shouldThrowIS { List joinableClauses = ImmutableList.of(factToCountryOnIsoCode(JoinType.LEFT)); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); Filter filter = new SelectorFilter("page", "this matches nothing"); new HashJoinSegmentStorageAdapter( @@ -2092,15 +1914,6 @@ public void test_makeCursors_originalFilterDoesNotMatchPreAnalysis_shouldThrowIS false, null ); - - /* - try { - - Assert.fail(); - } - catch (ISE e) { - Assert.assertTrue(e.getMessage().startsWith("Filter provided to cursor [")); - } - */ } + } diff --git a/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentTest.java b/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentTest.java index 458828670126..02da30855eb9 100644 --- a/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentTest.java +++ b/processing/src/test/java/org/apache/druid/segment/join/HashJoinSegmentTest.java @@ -25,6 +25,7 @@ import org.apache.druid.query.QueryContexts; import org.apache.druid.segment.QueryableIndexSegment; import org.apache.druid.segment.join.filter.rewrite.JoinFilterPreAnalysisGroup; +import org.apache.druid.segment.join.filter.rewrite.JoinFilterRewriteConfig; import org.apache.druid.segment.join.table.IndexedTableJoinable; import org.apache.druid.timeline.SegmentId; import org.hamcrest.CoreMatchers; @@ -41,6 +42,14 @@ public class HashJoinSegmentTest { + private JoinFilterRewriteConfig DEFAULT_JOIN_FILTER_REWRITE_CONFIG = new JoinFilterRewriteConfig( + true, + true, + true, + QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE, + false + ); + @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); @@ -80,10 +89,7 @@ public void setUp() throws IOException ); JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE + DEFAULT_JOIN_FILTER_REWRITE_CONFIG ); hashJoinSegment = new HashJoinSegment( @@ -102,10 +108,7 @@ public void test_constructor_noClauses() List joinableClauses = ImmutableList.of(); JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE + DEFAULT_JOIN_FILTER_REWRITE_CONFIG ); final HashJoinSegment ignored = new HashJoinSegment( diff --git a/processing/src/test/java/org/apache/druid/segment/join/JoinFilterAnalyzerTest.java b/processing/src/test/java/org/apache/druid/segment/join/JoinFilterAnalyzerTest.java index ef3411d4aacf..e77198464c3d 100644 --- a/processing/src/test/java/org/apache/druid/segment/join/JoinFilterAnalyzerTest.java +++ b/processing/src/test/java/org/apache/druid/segment/join/JoinFilterAnalyzerTest.java @@ -47,6 +47,7 @@ import org.apache.druid.segment.join.filter.JoinFilterSplit; import org.apache.druid.segment.join.filter.JoinableClauses; import org.apache.druid.segment.join.filter.rewrite.JoinFilterPreAnalysisGroup; +import org.apache.druid.segment.join.filter.rewrite.JoinFilterRewriteConfig; import org.apache.druid.segment.join.lookup.LookupJoinable; import org.apache.druid.segment.join.table.IndexedTableJoinable; import org.apache.druid.segment.virtual.ExpressionVirtualColumn; @@ -67,7 +68,7 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterOnChannel() regionToCountry(JoinType.LEFT) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), @@ -112,7 +113,11 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterOnChannel() null, ImmutableSet.of() ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + originalFilter, + joinableClauses, + VirtualColumns.EMPTY + ); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @@ -141,7 +146,7 @@ public void test_filterPushDown_factToRegionExprToCountryLeftFilterOnCountryName regionExprToCountry ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), @@ -173,7 +178,11 @@ public void test_filterPushDown_factToRegionExprToCountryLeftFilterOnCountryName new SelectorFilter("rtc.countryName", "United States"), ImmutableSet.of() ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + originalFilter, + joinableClauses, + VirtualColumns.EMPTY + ); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @@ -193,7 +202,7 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterOnChannelAndCount regionToCountry(JoinType.LEFT) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), @@ -234,7 +243,11 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterOnChannelAndCount new SelectorFilter("rtc.countryName", "United States"), ImmutableSet.of() ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + originalFilter, + joinableClauses, + VirtualColumns.EMPTY + ); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @@ -256,7 +269,7 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterOnNullColumns() regionToCountry(JoinType.LEFT) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), @@ -301,7 +314,12 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterOnNullColumns() ), ImmutableSet.of() ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + originalFilter, + joinableClauses, + VirtualColumns.EMPTY + ); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @@ -323,7 +341,7 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterOnInvalidColumns( ) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( @@ -360,7 +378,11 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterOnInvalidColumns( ), ImmutableSet.of() ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + originalFilter, + joinableClauses, + VirtualColumns.EMPTY + ); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @@ -379,7 +401,7 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterOnChannelVirtualC ) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), @@ -435,7 +457,11 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterOnChannelVirtualC null, ImmutableSet.of() ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + originalFilter, + joinableClauses, + virtualColumns + ); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @@ -461,7 +487,7 @@ public void test_filterPushDown_factToRegionFilterOnRHSRegionNameExprVirtualColu factToRegion(JoinType.LEFT) )); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), @@ -492,7 +518,11 @@ public void test_filterPushDown_factToRegionFilterOnRHSRegionNameExprVirtualColu new SelectorFilter("v0", "VIRGINIA"), ImmutableSet.of() ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + originalFilter, + joinableClauses.getJoinableClauses(), + virtualColumns + ); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @@ -562,7 +592,7 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterNormalizedAlready regionToCountry(JoinType.LEFT) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), @@ -653,7 +683,11 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterNormalizedAlready ), ImmutableSet.of() ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + originalFilter, + joinableClauses, + VirtualColumns.EMPTY + ); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @@ -686,7 +720,7 @@ public void test_filterPushDown_factExpressionsToRegionToCountryLeftFilterOnChan ) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), @@ -723,7 +757,11 @@ public void test_filterPushDown_factExpressionsToRegionToCountryLeftFilterOnChan new SelectorFilter("rtc.countryName", "States United"), ImmutableSet.of() ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + originalFilter, + joinableClauses, + VirtualColumns.EMPTY + ); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); ExpressionVirtualColumn expectedVirtualColumn = new ExpressionVirtualColumn( @@ -774,7 +812,7 @@ public void test_filterPushDown_factToRegionToCountryNotEquiJoinLeftFilterOnChan ) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("Cannot build hash-join matcher on non-equi-join condition: \"r1.regionIsoCode\" == regionIsoCode && reverse(\"r1.countryIsoCode\") == countryIsoCode"); @@ -823,7 +861,7 @@ public void test_filterPushDown_factToRegionToCountryLeftUnnormalizedFilter() ) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), @@ -891,7 +929,12 @@ public void test_filterPushDown_factToRegionToCountryLeftUnnormalizedFilter() ), ImmutableSet.of() ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + originalFilter, + joinableClauses, + VirtualColumns.EMPTY + ); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @@ -922,7 +965,7 @@ public void test_filterPushDown_factConcatExpressionToCountryLeftFilterOnChannel ) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), @@ -968,7 +1011,11 @@ public void test_filterPushDown_factConcatExpressionToCountryLeftFilterOnChannel expectedVirtualColumn ) ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(filter); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + filter, + joinableClauses, + VirtualColumns.EMPTY + ); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals( expectedFilterSplit.getBaseTableFilter(), @@ -1009,7 +1056,7 @@ public void test_filterPushDown_factConcatExpressionToCountryLeftFilterOnChannel ) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), @@ -1055,7 +1102,11 @@ public void test_filterPushDown_factConcatExpressionToCountryLeftFilterOnChannel expectedVirtualColumn ) ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(filter); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + filter, + joinableClauses, + VirtualColumns.EMPTY + ); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals( expectedFilterSplit.getBaseTableFilter(), @@ -1080,7 +1131,7 @@ public void test_filterPushDown_factToCountryRightWithFilterOnChannelAndJoinable new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "countryName", "Germany") ) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, @@ -1119,7 +1170,11 @@ public void test_filterPushDown_factToCountryRightWithFilterOnChannelAndJoinable new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "countryName", "Germany"), ImmutableSet.of() ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + originalFilter, + joinableClauses, + VirtualColumns.EMPTY + ); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @@ -1134,7 +1189,7 @@ public void test_filterPushDown_factToCountryRightWithFilterOnChannelAndJoinable new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "v", "Germany") ) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), @@ -1173,7 +1228,11 @@ public void test_filterPushDown_factToCountryRightWithFilterOnChannelAndJoinable new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "v", "Germany"), ImmutableSet.of() ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + originalFilter, + joinableClauses, + VirtualColumns.EMPTY + ); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @@ -1188,7 +1247,7 @@ public void test_filterPushDown_factToCountryRightWithFilterOnNullColumns() new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "countryName", null) ) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), @@ -1226,7 +1285,11 @@ public void test_filterPushDown_factToCountryRightWithFilterOnNullColumns() ), ImmutableSet.of() ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + originalFilter, + joinableClauses, + VirtualColumns.EMPTY + ); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @@ -1241,7 +1304,7 @@ public void test_filterPushDown_factToCountryRightWithFilterOnValueThatMatchesNo new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "countryName", "NO MATCH") ) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), @@ -1280,7 +1343,11 @@ public void test_filterPushDown_factToCountryRightWithFilterOnValueThatMatchesNo ), ImmutableSet.of() ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + originalFilter, + joinableClauses, + VirtualColumns.EMPTY + ); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @@ -1295,7 +1362,7 @@ public void test_filterPushDown_factToCountryRightWithFilterOnNullColumnsUsingLo new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "v", null) ) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), @@ -1332,7 +1399,11 @@ public void test_filterPushDown_factToCountryRightWithFilterOnNullColumnsUsingLo ), ImmutableSet.of() ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + originalFilter, + joinableClauses, + VirtualColumns.EMPTY + ); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @@ -1347,7 +1418,7 @@ public void test_filterPushDown_factToCountryInnerUsingCountryNumberFilterOnChan new SelectorFilter(FACT_TO_COUNTRY_ON_NUMBER_PREFIX + "countryName", "Australia") ) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), @@ -1393,7 +1464,11 @@ public void test_filterPushDown_factToCountryInnerUsingCountryNumberFilterOnChan new SelectorFilter(FACT_TO_COUNTRY_ON_NUMBER_PREFIX + "countryName", "Australia"), ImmutableSet.of() ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + originalFilter, + joinableClauses, + VirtualColumns.EMPTY + ); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @@ -1408,7 +1483,7 @@ public void test_filterPushDown_factToCountryInnerUsingCountryNumberFilterOnChan new SelectorFilter(FACT_TO_COUNTRY_ON_NUMBER_PREFIX + "v", "Australia") ) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), @@ -1453,7 +1528,11 @@ public void test_filterPushDown_factToCountryInnerUsingCountryNumberFilterOnChan new SelectorFilter(FACT_TO_COUNTRY_ON_NUMBER_PREFIX + "v", "Australia"), ImmutableSet.of() ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + originalFilter, + joinableClauses, + VirtualColumns.EMPTY + ); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @@ -1468,7 +1547,7 @@ public void test_filterPushDown_factToCountryInnerUsingCountryNumberFilterOnNull new SelectorFilter(FACT_TO_COUNTRY_ON_NUMBER_PREFIX + "countryName", null) ) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), @@ -1505,7 +1584,11 @@ public void test_filterPushDown_factToCountryInnerUsingCountryNumberFilterOnNull ), ImmutableSet.of() ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + originalFilter, + joinableClauses, + VirtualColumns.EMPTY + ); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @@ -1520,7 +1603,7 @@ public void test_filterPushDown_factToCountryInnerUsingCountryNumberFilterOnNull new SelectorFilter(FACT_TO_COUNTRY_ON_NUMBER_PREFIX + "v", null) ) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), @@ -1556,7 +1639,11 @@ public void test_filterPushDown_factToCountryInnerUsingCountryNumberFilterOnNull ), ImmutableSet.of() ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + originalFilter, + joinableClauses, + VirtualColumns.EMPTY + ); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @@ -1571,7 +1658,7 @@ public void test_filterPushDown_factToCountryFullWithFilterOnChannelAndCountryNa ) ); List joinableClauses = ImmutableList.of(factToCountryOnIsoCode(JoinType.FULL)); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), @@ -1611,7 +1698,11 @@ public void test_filterPushDown_factToCountryFullWithFilterOnChannelAndCountryNa new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "countryName", "El Salvador"), ImmutableSet.of() ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(filter); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + filter, + joinableClauses, + VirtualColumns.EMPTY + ); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @@ -1626,7 +1717,7 @@ public void test_filterPushDown_factToCountryFullWithFilterOnChannelAndCountryNa ) ); List joinableClauses = ImmutableList.of(factToCountryNameUsingIsoCodeLookup(JoinType.FULL)); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), @@ -1665,7 +1756,11 @@ public void test_filterPushDown_factToCountryFullWithFilterOnChannelAndCountryNa new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "v", "El Salvador"), ImmutableSet.of() ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(filter); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + filter, + joinableClauses, + VirtualColumns.EMPTY + ); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @@ -1680,7 +1775,7 @@ public void test_filterPushDown_factToCountryFullWithFilterOnNulls() new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "countryName", null) ) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), @@ -1718,7 +1813,11 @@ public void test_filterPushDown_factToCountryFullWithFilterOnNulls() ), ImmutableSet.of() ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + originalFilter, + joinableClauses, + VirtualColumns.EMPTY + ); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @@ -1733,7 +1832,7 @@ public void test_filterPushDown_factToCountryFullWithFilterOnNullsUsingLookup() new SelectorFilter(FACT_TO_COUNTRY_ON_ISO_CODE_PREFIX + "v", null) ) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, @@ -1769,7 +1868,11 @@ public void test_filterPushDown_factToCountryFullWithFilterOnNullsUsingLookup() ), ImmutableSet.of() ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + originalFilter, + joinableClauses, + VirtualColumns.EMPTY + ); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @@ -1796,7 +1899,7 @@ public void test_filterPushDown_factToRegionTwoColumnsToOneRHSColumnAndFilterOnR ); Filter originalFilter = new SelectorFilter("r1.regionName", "Fourems Province"); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), @@ -1832,7 +1935,12 @@ public void test_filterPushDown_factToRegionTwoColumnsToOneRHSColumnAndFilterOnR new SelectorFilter("r1.regionName", "Fourems Province"), ImmutableSet.of() ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + originalFilter, + joinableClauses, + VirtualColumns.EMPTY + ); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @@ -1864,7 +1972,7 @@ public void test_filterPushDown_factToRegionOneColumnToTwoRHSColumnsAndFilterOnR ) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), @@ -1900,7 +2008,12 @@ public void test_filterPushDown_factToRegionOneColumnToTwoRHSColumnsAndFilterOnR originalFilter, ImmutableSet.of() ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + originalFilter, + joinableClauses, + VirtualColumns.EMPTY + ); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @@ -1934,7 +2047,7 @@ public void test_filterPushDown_factToRegionThreeRHSColumnsAllDirectAndFilterOnR ) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), @@ -1975,7 +2088,11 @@ public void test_filterPushDown_factToRegionThreeRHSColumnsAllDirectAndFilterOnR null, ImmutableSet.of() ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + originalFilter, + joinableClauses, + VirtualColumns.EMPTY + ); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @@ -1990,10 +2107,13 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterOnPageDisablePush Filter originalFilter = new SelectorFilter("page", "Peremptory norm"); JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - false, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE + new JoinFilterRewriteConfig( + false, + true, + true, + QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE, + false + ) ); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( @@ -2026,7 +2146,11 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterOnPageDisablePush new SelectorFilter("page", "Peremptory norm"), ImmutableSet.of() ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + originalFilter, + joinableClauses.getJoinableClauses(), + VirtualColumns.EMPTY + ); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @@ -2061,10 +2185,13 @@ public void test_filterPushDown_factToRegionToCountryLeftEnablePushDownDisableRe ) ); JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( - true, - false, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE + new JoinFilterRewriteConfig( + true, + false, + true, + QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE, + false + ) ); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), @@ -2124,7 +2251,11 @@ public void test_filterPushDown_factToRegionToCountryLeftEnablePushDownDisableRe ), ImmutableSet.of() ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + originalFilter, + joinableClauses.getJoinableClauses(), + VirtualColumns.EMPTY + ); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @@ -2234,7 +2365,7 @@ public boolean supportsRequiredColumnRewrite() regionToCountry(JoinType.LEFT) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), joinableClauses, @@ -2366,7 +2497,11 @@ public boolean supportsRequiredColumnRewrite() ), expectedVirtualColumns ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + originalFilter, + joinableClauses, + VirtualColumns.EMPTY + ); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @@ -2386,7 +2521,7 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterOnTwoRHSColumnsSa regionToCountry(JoinType.LEFT) ); - JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = simplePreAnalysisGroup(); + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = makeDefaultConfigPreAnalysisGroup(); HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( factSegment.asStorageAdapter(), @@ -2432,7 +2567,86 @@ public void test_filterPushDown_factToRegionToCountryLeftFilterOnTwoRHSColumnsSa originalFilter, ImmutableSet.of() ); - JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalyses().get(originalFilter); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getAnalysis( + originalFilter, + joinableClauses, + VirtualColumns.EMPTY + ); + JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); + Assert.assertEquals(expectedFilterSplit, actualFilterSplit); + } + + + @Test + public void test_filterPushDown_factToRegionExprToCountryLeftFilterOnCountryNameWithOldRewriteMode() + { + Filter originalFilter = new SelectorFilter("rtc.countryName", "United States"); + JoinableClause regionExprToCountry = new JoinableClause( + REGION_TO_COUNTRY_PREFIX, + new IndexedTableJoinable(countriesTable), + JoinType.LEFT, + JoinConditionAnalysis.forExpression( + StringUtils.format( + "reverse(\"%scountryIsoCode\") == \"%scountryIsoCode\"", + FACT_TO_REGION_PREFIX, + REGION_TO_COUNTRY_PREFIX + ), + REGION_TO_COUNTRY_PREFIX, + ExprMacroTable.nil() + ) + ); + List joinableClauses = ImmutableList.of( + factToRegion(JoinType.LEFT), + regionExprToCountry + ); + + JoinFilterPreAnalysisGroup joinFilterPreAnalysisGroup = new JoinFilterPreAnalysisGroup( + new JoinFilterRewriteConfig( + true, + true, + true, + QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE, + true + ) + ); + + joinFilterPreAnalysisGroup.performAnalysisForOldRewriteMode( + originalFilter, + joinableClauses, + VirtualColumns.EMPTY + ); + + HashJoinSegmentStorageAdapter adapter = new HashJoinSegmentStorageAdapter( + factSegment.asStorageAdapter(), + joinableClauses, + joinFilterPreAnalysisGroup + ); + + JoinTestHelper.verifyCursors( + adapter.makeCursors( + originalFilter, + Intervals.ETERNITY, + VirtualColumns.EMPTY, + Granularities.ALL, + false, + null + ), + ImmutableList.of( + "page", + FACT_TO_REGION_PREFIX + "regionName", + REGION_TO_COUNTRY_PREFIX + "countryName" + ), + ImmutableList.of( + new Object[]{"Cream Soda", "Ainigriv", "United States"} + ) + ); + + JoinFilterSplit expectedFilterSplit = new JoinFilterSplit( + null, + new SelectorFilter("rtc.countryName", "United States"), + ImmutableSet.of() + ); + JoinFilterPreAnalysis joinFilterPreAnalysis = joinFilterPreAnalysisGroup.getPreAnalysisForOldRewriteMode(); JoinFilterSplit actualFilterSplit = JoinFilterAnalyzer.splitFilter(joinFilterPreAnalysis); Assert.assertEquals(expectedFilterSplit, actualFilterSplit); } @@ -2445,15 +2659,4 @@ public void test_JoinFilterSplit_equals() .withNonnullFields("baseTableFilter", "pushDownVirtualColumns") .verify(); } - - private static JoinFilterPreAnalysisGroup simplePreAnalysisGroup() - { - return new JoinFilterPreAnalysisGroup( - true, - true, - true, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE - ); - - } } diff --git a/processing/src/test/java/org/apache/druid/segment/join/JoinablesTest.java b/processing/src/test/java/org/apache/druid/segment/join/JoinablesTest.java index 2f0b59550be6..cf33f33c9547 100644 --- a/processing/src/test/java/org/apache/druid/segment/join/JoinablesTest.java +++ b/processing/src/test/java/org/apache/druid/segment/join/JoinablesTest.java @@ -29,6 +29,7 @@ import org.apache.druid.query.planning.PreJoinableClause; import org.apache.druid.segment.Segment; import org.apache.druid.segment.column.ColumnHolder; +import org.apache.druid.segment.join.filter.rewrite.JoinFilterRewriteConfig; import org.apache.druid.segment.join.lookup.LookupJoinable; import org.junit.Assert; import org.junit.Rule; @@ -43,6 +44,14 @@ public class JoinablesTest { + private static final JoinFilterRewriteConfig DEFAULT_JOIN_FILTER_REWRITE_CONFIG = new JoinFilterRewriteConfig( + QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_PUSH_DOWN, + QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE, + QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_VALUE_COLUMN_FILTERS, + QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE, + QueryContexts.DEFAULT_JOIN_FILTER_REWRITE_USE_OLD_REWRITE_MODE + ); + @Rule public ExpectedException expectedException = ExpectedException.none(); @@ -98,10 +107,9 @@ public void test_createSegmentMapFn_noClauses() ImmutableList.of(), NoopJoinableFactory.INSTANCE, new AtomicLong(), - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_PUSH_DOWN, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_VALUE_COLUMN_FILTERS, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE + DEFAULT_JOIN_FILTER_REWRITE_CONFIG, + null, + null ); Assert.assertSame(Function.identity(), segmentMapFn); @@ -125,10 +133,9 @@ public void test_createSegmentMapFn_unusableClause() ImmutableList.of(clause), NoopJoinableFactory.INSTANCE, new AtomicLong(), - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_PUSH_DOWN, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_VALUE_COLUMN_FILTERS, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE + DEFAULT_JOIN_FILTER_REWRITE_CONFIG, + null, + null ); } @@ -160,10 +167,9 @@ public void test_createSegmentMapFn_usableClause() } }, new AtomicLong(), - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_PUSH_DOWN, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_VALUE_COLUMN_FILTERS, - QueryContexts.DEFAULT_ENABLE_JOIN_FILTER_REWRITE_MAX_SIZE + DEFAULT_JOIN_FILTER_REWRITE_CONFIG, + null, + null ); Assert.assertNotSame(Function.identity(), segmentMapFn); diff --git a/server/src/main/java/org/apache/druid/segment/realtime/appenderator/SinkQuerySegmentWalker.java b/server/src/main/java/org/apache/druid/segment/realtime/appenderator/SinkQuerySegmentWalker.java index 5f0859863d2d..ca96730b1748 100644 --- a/server/src/main/java/org/apache/druid/segment/realtime/appenderator/SinkQuerySegmentWalker.java +++ b/server/src/main/java/org/apache/druid/segment/realtime/appenderator/SinkQuerySegmentWalker.java @@ -60,6 +60,7 @@ import org.apache.druid.segment.Segment; import org.apache.druid.segment.join.JoinableFactory; import org.apache.druid.segment.join.Joinables; +import org.apache.druid.segment.join.filter.rewrite.JoinFilterRewriteConfig; import org.apache.druid.segment.realtime.FireHydrant; import org.apache.druid.segment.realtime.plumber.Sink; import org.apache.druid.timeline.SegmentId; @@ -170,15 +171,22 @@ public QueryRunner getQueryRunnerForSegments(final Query query, final throw new ISE("Cannot handle subquery: %s", analysis.getDataSource()); } + final JoinFilterRewriteConfig joinFilterRewriteConfig = new JoinFilterRewriteConfig( + QueryContexts.getEnableJoinFilterPushDown(query), + QueryContexts.getEnableJoinFilterRewrite(query), + QueryContexts.getEnableJoinFilterRewriteValueColumnFilters(query), + QueryContexts.getJoinFilterRewriteMaxSize(query), + QueryContexts.getUseJoinFilterRewriteOldRewriteMode(query) + ); + // segmentMapFn maps each base Segment into a joined Segment if necessary. final Function segmentMapFn = Joinables.createSegmentMapFn( analysis.getPreJoinableClauses(), joinableFactory, cpuTimeAccumulator, - QueryContexts.getEnableJoinFilterPushDown(query), - QueryContexts.getEnableJoinFilterRewrite(query), - QueryContexts.getEnableJoinFilterRewriteValueColumnFilters(query), - QueryContexts.getJoinFilterRewriteMaxSize(query) + joinFilterRewriteConfig, + query.getFilter() == null ? null : query.getFilter().toFilter(), + query.getVirtualColumns() ); Iterable> perSegmentRunners = Iterables.transform( diff --git a/server/src/main/java/org/apache/druid/server/LocalQuerySegmentWalker.java b/server/src/main/java/org/apache/druid/server/LocalQuerySegmentWalker.java index ee453d9e07c5..174daae75f23 100644 --- a/server/src/main/java/org/apache/druid/server/LocalQuerySegmentWalker.java +++ b/server/src/main/java/org/apache/druid/server/LocalQuerySegmentWalker.java @@ -39,6 +39,7 @@ import org.apache.druid.segment.SegmentWrangler; import org.apache.druid.segment.join.JoinableFactory; import org.apache.druid.segment.join.Joinables; +import org.apache.druid.segment.join.filter.rewrite.JoinFilterRewriteConfig; import org.joda.time.Interval; import java.util.HashSet; @@ -93,14 +94,20 @@ public QueryRunner getQueryRunnerForIntervals(final Query query, final final Query prioritizedAndLaned = prioritizeAndLaneQuery(query, segments); final AtomicLong cpuAccumulator = new AtomicLong(0L); + final JoinFilterRewriteConfig joinFilterRewriteConfig = new JoinFilterRewriteConfig( + QueryContexts.getEnableJoinFilterPushDown(prioritizedAndLaned), + QueryContexts.getEnableJoinFilterRewrite(prioritizedAndLaned), + QueryContexts.getEnableJoinFilterRewriteValueColumnFilters(prioritizedAndLaned), + QueryContexts.getJoinFilterRewriteMaxSize(prioritizedAndLaned), + QueryContexts.getUseJoinFilterRewriteOldRewriteMode(prioritizedAndLaned) + ); final Function segmentMapFn = Joinables.createSegmentMapFn( analysis.getPreJoinableClauses(), joinableFactory, cpuAccumulator, - QueryContexts.getEnableJoinFilterPushDown(prioritizedAndLaned), - QueryContexts.getEnableJoinFilterRewrite(prioritizedAndLaned), - QueryContexts.getEnableJoinFilterRewriteValueColumnFilters(prioritizedAndLaned), - QueryContexts.getJoinFilterRewriteMaxSize(prioritizedAndLaned) + joinFilterRewriteConfig, + prioritizedAndLaned.getFilter() == null ? null : prioritizedAndLaned.getFilter().toFilter(), + prioritizedAndLaned.getVirtualColumns() ); final QueryRunnerFactory> queryRunnerFactory = conglomerate.findFactory(prioritizedAndLaned); diff --git a/server/src/main/java/org/apache/druid/server/coordination/ServerManager.java b/server/src/main/java/org/apache/druid/server/coordination/ServerManager.java index 624b02093f0c..3d9b61b1f57e 100644 --- a/server/src/main/java/org/apache/druid/server/coordination/ServerManager.java +++ b/server/src/main/java/org/apache/druid/server/coordination/ServerManager.java @@ -58,6 +58,7 @@ import org.apache.druid.segment.Segment; import org.apache.druid.segment.join.JoinableFactory; import org.apache.druid.segment.join.Joinables; +import org.apache.druid.segment.join.filter.rewrite.JoinFilterRewriteConfig; import org.apache.druid.server.SegmentManager; import org.apache.druid.server.SetAndVerifyContextQueryRunner; import org.apache.druid.server.initialization.ServerConfig; @@ -193,15 +194,22 @@ public QueryRunner getQueryRunnerForSegments(Query query, Iterable(); } + final JoinFilterRewriteConfig joinFilterRewriteConfig = new JoinFilterRewriteConfig( + QueryContexts.getEnableJoinFilterPushDown(query), + QueryContexts.getEnableJoinFilterRewrite(query), + QueryContexts.getEnableJoinFilterRewriteValueColumnFilters(query), + QueryContexts.getJoinFilterRewriteMaxSize(query), + QueryContexts.getUseJoinFilterRewriteOldRewriteMode(query) + ); + // segmentMapFn maps each base Segment into a joined Segment if necessary. final Function segmentMapFn = Joinables.createSegmentMapFn( analysis.getPreJoinableClauses(), joinableFactory, cpuTimeAccumulator, - QueryContexts.getEnableJoinFilterPushDown(query), - QueryContexts.getEnableJoinFilterRewrite(query), - QueryContexts.getEnableJoinFilterRewriteValueColumnFilters(query), - QueryContexts.getJoinFilterRewriteMaxSize(query) + joinFilterRewriteConfig, + query.getFilter() == null ? null : query.getFilter().toFilter(), + query.getVirtualColumns() ); FunctionalIterable> queryRunners = FunctionalIterable diff --git a/server/src/test/java/org/apache/druid/server/TestClusterQuerySegmentWalker.java b/server/src/test/java/org/apache/druid/server/TestClusterQuerySegmentWalker.java index e23780ae314f..d0697abc29eb 100644 --- a/server/src/test/java/org/apache/druid/server/TestClusterQuerySegmentWalker.java +++ b/server/src/test/java/org/apache/druid/server/TestClusterQuerySegmentWalker.java @@ -47,6 +47,7 @@ import org.apache.druid.segment.Segment; import org.apache.druid.segment.join.JoinableFactory; import org.apache.druid.segment.join.Joinables; +import org.apache.druid.segment.join.filter.rewrite.JoinFilterRewriteConfig; import org.apache.druid.timeline.TimelineObjectHolder; import org.apache.druid.timeline.VersionedIntervalTimeline; import org.apache.druid.timeline.partition.PartitionChunk; @@ -139,14 +140,21 @@ public QueryRunner getQueryRunnerForSegments(final Query query, final throw new ISE("Cannot handle subquery: %s", analysis.getDataSource()); } + final JoinFilterRewriteConfig joinFilterRewriteConfig = new JoinFilterRewriteConfig( + QueryContexts.getEnableJoinFilterPushDown(query), + QueryContexts.getEnableJoinFilterRewrite(query), + QueryContexts.getEnableJoinFilterRewriteValueColumnFilters(query), + QueryContexts.getJoinFilterRewriteMaxSize(query), + QueryContexts.getUseJoinFilterRewriteOldRewriteMode(query) + ); + final Function segmentMapFn = Joinables.createSegmentMapFn( analysis.getPreJoinableClauses(), joinableFactory, new AtomicLong(), - QueryContexts.getEnableJoinFilterPushDown(query), - QueryContexts.getEnableJoinFilterRewrite(query), - QueryContexts.getEnableJoinFilterRewriteValueColumnFilters(query), - QueryContexts.getJoinFilterRewriteMaxSize(query) + joinFilterRewriteConfig, + query.getFilter() == null ? null : query.getFilter().toFilter(), + query.getVirtualColumns() ); final QueryRunner baseRunner = new FinalizeResultsQueryRunner<>( From d74ff79ccdac1d239836ecc48cb45834d5449f3f Mon Sep 17 00:00:00 2001 From: jon-wei Date: Fri, 5 Jun 2020 23:30:00 -0700 Subject: [PATCH 6/9] Fixes --- .../benchmark/JoinAndLookupBenchmark.java | 45 ++++++++++++------- .../druid/query/filter/InDimFilter.java | 32 ++++++------- .../apache/druid/segment/filter/InFilter.java | 16 +++---- .../rewrite/JoinFilterPreAnalysisGroup.java | 1 - 4 files changed, 53 insertions(+), 41 deletions(-) diff --git a/benchmarks/src/test/java/org/apache/druid/benchmark/JoinAndLookupBenchmark.java b/benchmarks/src/test/java/org/apache/druid/benchmark/JoinAndLookupBenchmark.java index 4c707380d32a..c084e8c3b6e2 100644 --- a/benchmarks/src/test/java/org/apache/druid/benchmark/JoinAndLookupBenchmark.java +++ b/benchmarks/src/test/java/org/apache/druid/benchmark/JoinAndLookupBenchmark.java @@ -49,6 +49,7 @@ import org.apache.druid.segment.join.JoinType; import org.apache.druid.segment.join.JoinableClause; import org.apache.druid.segment.join.filter.rewrite.JoinFilterPreAnalysisGroup; +import org.apache.druid.segment.join.filter.rewrite.JoinFilterRewriteConfig; import org.apache.druid.segment.join.lookup.LookupJoinable; import org.apache.druid.segment.join.table.IndexedTableJoinable; import org.apache.druid.segment.virtual.ExpressionVirtualColumn; @@ -139,10 +140,13 @@ public void setup() throws IOException ) ); JoinFilterPreAnalysisGroup preAnalysisGroupLookupStringKey = new JoinFilterPreAnalysisGroup( - false, - false, - false, - 0 + new JoinFilterRewriteConfig( + false, + false, + false, + 0, + false + ) ); hashJoinLookupStringKeySegment = new HashJoinSegment( @@ -165,10 +169,13 @@ public void setup() throws IOException ); JoinFilterPreAnalysisGroup preAnalysisGroupLookupLongKey = new JoinFilterPreAnalysisGroup( - false, - false, - false, - 0 + new JoinFilterRewriteConfig( + false, + false, + false, + 0, + false + ) ); hashJoinLookupLongKeySegment = new HashJoinSegment( baseSegment, @@ -190,10 +197,13 @@ public void setup() throws IOException ); JoinFilterPreAnalysisGroup preAnalysisGroupIndexedStringKey = new JoinFilterPreAnalysisGroup( - false, - false, - false, - 0 + new JoinFilterRewriteConfig( + false, + false, + false, + 0, + false + ) ); hashJoinIndexedTableStringKeySegment = new HashJoinSegment( baseSegment, @@ -214,10 +224,13 @@ public void setup() throws IOException ) ); JoinFilterPreAnalysisGroup preAnalysisGroupIndexedLongKey = new JoinFilterPreAnalysisGroup( - false, - false, - false, - 0 + new JoinFilterRewriteConfig( + false, + false, + false, + 0, + false + ) ); hashJoinIndexedTableLongKeySegment = new HashJoinSegment( baseSegment, 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 ffa75991d75d..42148ad33798 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 @@ -168,21 +168,6 @@ public byte[] getCacheKey() return cacheKey; } - private static HashCode computeValuesHashCode(Set values) - { - final List sortedValues = new ArrayList<>(values); - sortedValues.sort(Comparator.nullsFirst(Ordering.natural())); - final Hasher hasher = Hashing.sha256().newHasher(); - for (String v : sortedValues) { - if (v == null) { - hasher.putInt(0); - } else { - hasher.putString(v, StandardCharsets.UTF_8); - } - } - return hasher.hash(); - } - @Override public DimFilter optimize() { @@ -316,7 +301,22 @@ public int hashCode() if (valuesHashCode == null) { valuesHashCode = computeValuesHashCode(values); } - return Objects.hash(values, dimension, extractionFn, filterTuning); + return Objects.hash(valuesHashCode, dimension, extractionFn, filterTuning); + } + + public static HashCode computeValuesHashCode(Set values) + { + final List sortedValues = new ArrayList<>(values); + sortedValues.sort(Comparator.nullsFirst(Ordering.natural())); + final Hasher hasher = Hashing.sha256().newHasher(); + for (String v : sortedValues) { + if (v == null) { + hasher.putInt(0); + } else { + hasher.putString(v, StandardCharsets.UTF_8); + } + } + return hasher.hash(); } private DruidLongPredicate createLongPredicate() diff --git a/processing/src/main/java/org/apache/druid/segment/filter/InFilter.java b/processing/src/main/java/org/apache/druid/segment/filter/InFilter.java index 848730536656..b631c2de8527 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/InFilter.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/InFilter.java @@ -37,6 +37,7 @@ import org.apache.druid.query.filter.DruidPredicateFactory; import org.apache.druid.query.filter.Filter; import org.apache.druid.query.filter.FilterTuning; +import org.apache.druid.query.filter.InDimFilter; import org.apache.druid.query.filter.ValueMatcher; import org.apache.druid.query.filter.vector.VectorValueMatcher; import org.apache.druid.query.filter.vector.VectorValueMatcherColumnProcessorFactory; @@ -73,7 +74,7 @@ public class InFilter implements Filter private final Supplier longPredicateSupplier; private final Supplier floatPredicateSupplier; private final Supplier doublePredicateSupplier; - private final HashCode valuesHashCode; + private HashCode valuesHashCode; public InFilter( String dimension, @@ -235,8 +236,7 @@ private DruidPredicateFactory getPredicateFactory() values, longPredicateSupplier, floatPredicateSupplier, - doublePredicateSupplier, - valuesHashCode + doublePredicateSupplier ); } @@ -259,6 +259,9 @@ public boolean equals(Object o) @Override public int hashCode() { + if (valuesHashCode == null) { + valuesHashCode = InDimFilter.computeValuesHashCode(values); + } return Objects.hash(dimension, valuesHashCode, extractionFn, filterTuning); } @@ -270,15 +273,13 @@ static class InFilterDruidPredicateFactory implements DruidPredicateFactory private final Supplier longPredicateSupplier; private final Supplier floatPredicateSupplier; private final Supplier doublePredicateSupplier; - private final HashCode valuesHashCode; InFilterDruidPredicateFactory( ExtractionFn extractionFn, Set values, Supplier longPredicateSupplier, Supplier floatPredicateSupplier, - Supplier doublePredicateSupplier, - HashCode valuesHashCode + Supplier doublePredicateSupplier ) { this.extractionFn = extractionFn; @@ -286,7 +287,6 @@ static class InFilterDruidPredicateFactory implements DruidPredicateFactory this.longPredicateSupplier = longPredicateSupplier; this.floatPredicateSupplier = floatPredicateSupplier; this.doublePredicateSupplier = doublePredicateSupplier; - this.valuesHashCode = valuesHashCode; } @Override @@ -345,7 +345,7 @@ public boolean equals(Object o) @Override public int hashCode() { - return Objects.hash(extractionFn, valuesHashCode); + return Objects.hash(extractionFn, values); } } } diff --git a/processing/src/main/java/org/apache/druid/segment/join/filter/rewrite/JoinFilterPreAnalysisGroup.java b/processing/src/main/java/org/apache/druid/segment/join/filter/rewrite/JoinFilterPreAnalysisGroup.java index ae3f6fa228f1..e4e935ff15ab 100644 --- a/processing/src/main/java/org/apache/druid/segment/join/filter/rewrite/JoinFilterPreAnalysisGroup.java +++ b/processing/src/main/java/org/apache/druid/segment/join/filter/rewrite/JoinFilterPreAnalysisGroup.java @@ -87,7 +87,6 @@ public JoinFilterPreAnalysis computeJoinFilterPreAnalysisIfAbsent( } JoinFilterPreAnalysisGroupKey key = new JoinFilterPreAnalysisGroupKey(filter, clauses, virtualColumns); - return analyses.computeIfAbsent( key, (groupKey) -> { From 8024443484f1c79138d2ca36850f9b834e8b3efd Mon Sep 17 00:00:00 2001 From: jon-wei Date: Fri, 5 Jun 2020 23:58:02 -0700 Subject: [PATCH 7/9] Restore @Override --- .../main/java/org/apache/druid/query/groupby/GroupByQuery.java | 1 + .../src/main/java/org/apache/druid/query/scan/ScanQuery.java | 1 + .../java/org/apache/druid/query/timeseries/TimeseriesQuery.java | 1 + .../src/main/java/org/apache/druid/query/topn/TopNQuery.java | 1 + 4 files changed, 4 insertions(+) diff --git a/processing/src/main/java/org/apache/druid/query/groupby/GroupByQuery.java b/processing/src/main/java/org/apache/druid/query/groupby/GroupByQuery.java index 4c17f16b03af..462644425dc1 100644 --- a/processing/src/main/java/org/apache/druid/query/groupby/GroupByQuery.java +++ b/processing/src/main/java/org/apache/druid/query/groupby/GroupByQuery.java @@ -250,6 +250,7 @@ private List> verifySubtotalsSpec( } @JsonProperty + @Override public VirtualColumns getVirtualColumns() { return virtualColumns; 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 719f5f27e6f7..be5d24b71b34 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 @@ -184,6 +184,7 @@ private Integer validateAndGetMaxSegmentPartitionsOrderedInMemory() } @JsonProperty + @Override public VirtualColumns getVirtualColumns() { return virtualColumns; diff --git a/processing/src/main/java/org/apache/druid/query/timeseries/TimeseriesQuery.java b/processing/src/main/java/org/apache/druid/query/timeseries/TimeseriesQuery.java index a959737cf645..94c187346da5 100644 --- a/processing/src/main/java/org/apache/druid/query/timeseries/TimeseriesQuery.java +++ b/processing/src/main/java/org/apache/druid/query/timeseries/TimeseriesQuery.java @@ -104,6 +104,7 @@ public String getType() } @JsonProperty + @Override public VirtualColumns getVirtualColumns() { return virtualColumns; 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 c77011c0325d..f5bdec1982a0 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 @@ -114,6 +114,7 @@ public String getType() } @JsonProperty + @Override public VirtualColumns getVirtualColumns() { return virtualColumns; From d79b3019894b01f5e02df92754bad56fc3dc4679 Mon Sep 17 00:00:00 2001 From: jon-wei Date: Mon, 8 Jun 2020 16:24:28 -0700 Subject: [PATCH 8/9] Fix inspections and tests --- .../druid/query/filter/DimFilterUtils.java | 24 ------------------- .../druid/query/filter/InDimFilter.java | 19 ++++++++++----- .../druid/segment/filter/AndFilter.java | 4 +++- .../apache/druid/segment/filter/InFilter.java | 14 +++++++---- .../apache/druid/segment/filter/OrFilter.java | 4 +++- .../rewrite/JoinFilterPreAnalysisGroup.java | 15 ------------ .../druid/query/filter/InDimFilterTest.java | 12 ++++++++++ .../druid/segment/filter/AndFilterTest.java | 6 ++++- .../druid/segment/filter/InFilterTest.java | 2 +- 9 files changed, 47 insertions(+), 53 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/query/filter/DimFilterUtils.java b/processing/src/main/java/org/apache/druid/query/filter/DimFilterUtils.java index 3fcd719e25f5..f8e018eda9ca 100644 --- a/processing/src/main/java/org/apache/druid/query/filter/DimFilterUtils.java +++ b/processing/src/main/java/org/apache/druid/query/filter/DimFilterUtils.java @@ -23,7 +23,6 @@ import com.google.common.collect.RangeSet; import org.apache.druid.timeline.partition.ShardSpec; -import java.nio.ByteBuffer; import java.util.HashMap; import java.util.LinkedHashSet; import java.util.List; @@ -57,29 +56,6 @@ public class DimFilterUtils public static final byte STRING_SEPARATOR = (byte) 0xFF; - static byte[] computeCacheKey(byte cacheIdKey, List filters) - { - if (filters.size() == 1) { - return filters.get(0).getCacheKey(); - } - - byte[][] cacheKeys = new byte[filters.size()][]; - int totalSize = 0; - int index = 0; - for (DimFilter field : filters) { - cacheKeys[index] = field.getCacheKey(); - totalSize += cacheKeys[index].length; - ++index; - } - - ByteBuffer retVal = ByteBuffer.allocate(1 + totalSize); - retVal.put(cacheIdKey); - for (byte[] cacheKey : cacheKeys) { - retVal.put(cacheKey); - } - return retVal.array(); - } - /** * Filter the given iterable of objects by removing any object whose ShardSpec, obtained from the converter function, * does not fit in the RangeSet of the dimFilter {@link DimFilter#getDimensionRangeSet(String)}. The returned set 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 42148ad33798..844add072841 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 @@ -34,7 +34,6 @@ import com.google.common.collect.Range; import com.google.common.collect.RangeSet; import com.google.common.collect.TreeRangeSet; -import com.google.common.hash.HashCode; import com.google.common.hash.Hasher; import com.google.common.hash.Hashing; import it.unimi.dsi.fastutil.ints.IntArrayList; @@ -51,6 +50,7 @@ import org.apache.druid.segment.filter.InFilter; import javax.annotation.Nullable; +import javax.annotation.concurrent.Immutable; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; @@ -62,6 +62,7 @@ import java.util.Set; import java.util.stream.Collectors; +@Immutable public class InDimFilter implements DimFilter { // determined through benchmark that binary search on long[] is faster than HashSet until ~16 elements @@ -83,7 +84,7 @@ public class InDimFilter implements DimFilter private byte[] cacheKey; @JsonIgnore - private HashCode valuesHashCode; + private Integer valuesHashCode; @JsonCreator public InDimFilter( @@ -162,7 +163,7 @@ public byte[] getCacheKey() .appendByte(DimFilterUtils.STRING_SEPARATOR) .appendByteArray(extractionFn == null ? new byte[0] : extractionFn.getCacheKey()) .appendByte(DimFilterUtils.STRING_SEPARATOR) - .appendByteArray(valuesHashCode.asBytes()) + .appendInt(valuesHashCode) .build(); } return cacheKey; @@ -288,8 +289,14 @@ public boolean equals(Object o) if (o == null || getClass() != o.getClass()) { return false; } + InDimFilter that = (InDimFilter) o; - return values.equals(that.values) && + + // make sure the hashCode is initialized for both filters + hashCode(); + that.hashCode(); + + return valuesHashCode.equals(that.valuesHashCode) && dimension.equals(that.dimension) && Objects.equals(extractionFn, that.extractionFn) && Objects.equals(filterTuning, that.filterTuning); @@ -304,7 +311,7 @@ public int hashCode() return Objects.hash(valuesHashCode, dimension, extractionFn, filterTuning); } - public static HashCode computeValuesHashCode(Set values) + public static int computeValuesHashCode(Set values) { final List sortedValues = new ArrayList<>(values); sortedValues.sort(Comparator.nullsFirst(Ordering.natural())); @@ -316,7 +323,7 @@ public static HashCode computeValuesHashCode(Set values) hasher.putString(v, StandardCharsets.UTF_8); } } - return hasher.hash(); + return hasher.hash().asInt(); } private DruidLongPredicate createLongPredicate() diff --git a/processing/src/main/java/org/apache/druid/segment/filter/AndFilter.java b/processing/src/main/java/org/apache/druid/segment/filter/AndFilter.java index c0c05cc0aad8..d5d9068bc129 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/AndFilter.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/AndFilter.java @@ -39,6 +39,7 @@ import org.apache.druid.segment.ColumnSelectorFactory; import org.apache.druid.segment.vector.VectorColumnSelectorFactory; +import javax.annotation.concurrent.Immutable; import java.util.ArrayList; import java.util.HashSet; import java.util.List; @@ -47,6 +48,7 @@ /** */ +@Immutable public class AndFilter implements BooleanFilter { private static final Joiner AND_JOINER = Joiner.on(" && "); @@ -259,7 +261,7 @@ public boolean equals(Object o) return false; } AndFilter andFilter = (AndFilter) o; - return Objects.equals(getFilters(), andFilter.getFilters()); + return Objects.equals(hashCode(), andFilter.hashCode()); } @Override diff --git a/processing/src/main/java/org/apache/druid/segment/filter/InFilter.java b/processing/src/main/java/org/apache/druid/segment/filter/InFilter.java index b631c2de8527..6472b6ca350e 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/InFilter.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/InFilter.java @@ -23,7 +23,7 @@ import com.google.common.base.Predicate; import com.google.common.base.Supplier; import com.google.common.collect.ImmutableSet; -import com.google.common.hash.HashCode; +import com.google.errorprone.annotations.Immutable; import it.unimi.dsi.fastutil.ints.IntIterable; import it.unimi.dsi.fastutil.ints.IntIterator; import org.apache.druid.collections.bitmap.ImmutableBitmap; @@ -65,6 +65,7 @@ * In default null handling mode, this filter is equivalent to {@code (dimension IN [values])} or * {@code (dimension IN [non-null values, ''])} when {@link #values} contains nulls. */ +@Immutable public class InFilter implements Filter { private final String dimension; @@ -74,7 +75,7 @@ public class InFilter implements Filter private final Supplier longPredicateSupplier; private final Supplier floatPredicateSupplier; private final Supplier doublePredicateSupplier; - private HashCode valuesHashCode; + private Integer valuesHashCode; public InFilter( String dimension, @@ -84,7 +85,7 @@ public InFilter( Supplier doublePredicateSupplier, ExtractionFn extractionFn, FilterTuning filterTuning, - HashCode valuesHashCode + Integer valuesHashCode ) { this.dimension = dimension; @@ -250,8 +251,13 @@ public boolean equals(Object o) return false; } InFilter inFilter = (InFilter) o; + + // make sure the hashCode is initialized for both filters + hashCode(); + inFilter.hashCode(); + return Objects.equals(dimension, inFilter.dimension) && - Objects.equals(values, inFilter.values) && + Objects.equals(valuesHashCode, inFilter.valuesHashCode) && Objects.equals(extractionFn, inFilter.extractionFn) && Objects.equals(filterTuning, inFilter.filterTuning); } diff --git a/processing/src/main/java/org/apache/druid/segment/filter/OrFilter.java b/processing/src/main/java/org/apache/druid/segment/filter/OrFilter.java index 2ba9a985d15d..00d687f5b0c0 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/OrFilter.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/OrFilter.java @@ -39,6 +39,7 @@ import org.apache.druid.segment.ColumnSelectorFactory; import org.apache.druid.segment.vector.VectorColumnSelectorFactory; +import javax.annotation.concurrent.Immutable; import java.util.ArrayList; import java.util.HashSet; import java.util.List; @@ -47,6 +48,7 @@ /** */ +@Immutable public class OrFilter implements BooleanFilter { private static final Joiner OR_JOINER = Joiner.on(" || "); @@ -244,7 +246,7 @@ public boolean equals(Object o) return false; } OrFilter orFilter = (OrFilter) o; - return Objects.equals(getFilters(), orFilter.getFilters()); + return Objects.equals(hashCode(), orFilter.hashCode()); } @Override diff --git a/processing/src/main/java/org/apache/druid/segment/join/filter/rewrite/JoinFilterPreAnalysisGroup.java b/processing/src/main/java/org/apache/druid/segment/join/filter/rewrite/JoinFilterPreAnalysisGroup.java index e4e935ff15ab..d43fd10264f3 100644 --- a/processing/src/main/java/org/apache/druid/segment/join/filter/rewrite/JoinFilterPreAnalysisGroup.java +++ b/processing/src/main/java/org/apache/druid/segment/join/filter/rewrite/JoinFilterPreAnalysisGroup.java @@ -146,21 +146,6 @@ public JoinFilterPreAnalysisGroupKey( this.virtualColumns = virtualColumns; } - public Filter getFilter() - { - return filter; - } - - public List getClauses() - { - return clauses; - } - - public VirtualColumns getVirtualColumns() - { - return virtualColumns; - } - @Override public boolean equals(Object o) { diff --git a/processing/src/test/java/org/apache/druid/query/filter/InDimFilterTest.java b/processing/src/test/java/org/apache/druid/query/filter/InDimFilterTest.java index 9cf5b8f67bdb..b9605adcc1c7 100644 --- a/processing/src/test/java/org/apache/druid/query/filter/InDimFilterTest.java +++ b/processing/src/test/java/org/apache/druid/query/filter/InDimFilterTest.java @@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; +import nl.jqno.equalsverifier.EqualsVerifier; import org.apache.druid.common.config.NullHandling; import org.apache.druid.jackson.DefaultObjectMapper; import org.apache.druid.query.extraction.RegexDimExtractionFn; @@ -144,4 +145,15 @@ public void testOptimizeSingleValueInToSelector() final InDimFilter filter = new InDimFilter("dim", Collections.singleton("v1"), null); Assert.assertEquals(new SelectorDimFilter("dim", "v1", null), filter.optimize()); } + + @Test + public void test_equals() + { + EqualsVerifier.forClass(InDimFilter.class) + .usingGetClass() + .withNonnullFields("dimension", "values") + .withIgnoredFields("values", "cacheKey", "longPredicateSupplier", "floatPredicateSupplier", "doublePredicateSupplier") + .verify(); + } + } diff --git a/processing/src/test/java/org/apache/druid/segment/filter/AndFilterTest.java b/processing/src/test/java/org/apache/druid/segment/filter/AndFilterTest.java index 3af7bf89ec1c..1ff9bd5f878a 100644 --- a/processing/src/test/java/org/apache/druid/segment/filter/AndFilterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/filter/AndFilterTest.java @@ -180,6 +180,10 @@ public void testNotAnd() @Test public void test_equals() { - EqualsVerifier.forClass(AndFilter.class).usingGetClass().withNonnullFields("filters").verify(); + EqualsVerifier.forClass(AndFilter.class) + .usingGetClass() + .withNonnullFields("filters") + .withIgnoredFields("filters") + .verify(); } } diff --git a/processing/src/test/java/org/apache/druid/segment/filter/InFilterTest.java b/processing/src/test/java/org/apache/druid/segment/filter/InFilterTest.java index 6a0db8463feb..0be6cf5b0c50 100644 --- a/processing/src/test/java/org/apache/druid/segment/filter/InFilterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/filter/InFilterTest.java @@ -384,7 +384,7 @@ public void test_equals() EqualsVerifier.forClass(InFilter.class) .usingGetClass() .withNonnullFields("dimension", "values") - .withIgnoredFields("longPredicateSupplier", "floatPredicateSupplier", "doublePredicateSupplier") + .withIgnoredFields("values", "longPredicateSupplier", "floatPredicateSupplier", "doublePredicateSupplier") .verify(); } From b6bd57dcd59a3d8fd87ae29906ae917349d3c7d4 Mon Sep 17 00:00:00 2001 From: jon-wei Date: Mon, 8 Jun 2020 17:20:41 -0700 Subject: [PATCH 9/9] Memoize, change test name --- .../druid/query/filter/InDimFilter.java | 22 +++++-------------- .../druid/segment/filter/AndFilter.java | 11 +++++----- .../apache/druid/segment/filter/InFilter.java | 21 +++++++----------- .../apache/druid/segment/filter/OrFilter.java | 11 +++++----- .../rewrite/JoinFilterPreAnalysisGroup.java | 9 -------- .../druid/query/filter/InDimFilterTest.java | 6 +++-- .../druid/segment/filter/AndFilterTest.java | 9 +++++++- .../druid/segment/filter/InFilterTest.java | 5 ++++- .../druid/segment/filter/OrFilterTest.java | 16 ++++++++++++-- .../druid/sql/calcite/CalciteQueryTest.java | 2 +- 10 files changed, 54 insertions(+), 58 deletions(-) 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 844add072841..4fb3799ac291 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 @@ -50,7 +50,6 @@ import org.apache.druid.segment.filter.InFilter; import javax.annotation.Nullable; -import javax.annotation.concurrent.Immutable; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; @@ -62,7 +61,6 @@ import java.util.Set; import java.util.stream.Collectors; -@Immutable public class InDimFilter implements DimFilter { // determined through benchmark that binary search on long[] is faster than HashSet until ~16 elements @@ -84,7 +82,7 @@ public class InDimFilter implements DimFilter private byte[] cacheKey; @JsonIgnore - private Integer valuesHashCode; + private final Supplier valuesHashCode; @JsonCreator public InDimFilter( @@ -112,6 +110,7 @@ public InDimFilter( this.longPredicateSupplier = getLongPredicateSupplier(); this.floatPredicateSupplier = getFloatPredicateSupplier(); this.doublePredicateSupplier = getDoublePredicateSupplier(); + this.valuesHashCode = Suppliers.memoize(() -> computeValuesHashCode(values)); } /** @@ -154,16 +153,13 @@ public FilterTuning getFilterTuning() @Override public byte[] getCacheKey() { - if (valuesHashCode == null) { - valuesHashCode = computeValuesHashCode(values); - } if (cacheKey == null) { cacheKey = new CacheKeyBuilder(DimFilterUtils.IN_CACHE_ID) .appendString(dimension) .appendByte(DimFilterUtils.STRING_SEPARATOR) .appendByteArray(extractionFn == null ? new byte[0] : extractionFn.getCacheKey()) .appendByte(DimFilterUtils.STRING_SEPARATOR) - .appendInt(valuesHashCode) + .appendInt(valuesHashCode.get()) .build(); } return cacheKey; @@ -291,12 +287,7 @@ public boolean equals(Object o) } InDimFilter that = (InDimFilter) o; - - // make sure the hashCode is initialized for both filters - hashCode(); - that.hashCode(); - - return valuesHashCode.equals(that.valuesHashCode) && + return valuesHashCode.get().equals(that.valuesHashCode.get()) && dimension.equals(that.dimension) && Objects.equals(extractionFn, that.extractionFn) && Objects.equals(filterTuning, that.filterTuning); @@ -305,10 +296,7 @@ public boolean equals(Object o) @Override public int hashCode() { - if (valuesHashCode == null) { - valuesHashCode = computeValuesHashCode(values); - } - return Objects.hash(valuesHashCode, dimension, extractionFn, filterTuning); + return Objects.hash(valuesHashCode.get(), dimension, extractionFn, filterTuning); } public static int computeValuesHashCode(Set values) diff --git a/processing/src/main/java/org/apache/druid/segment/filter/AndFilter.java b/processing/src/main/java/org/apache/druid/segment/filter/AndFilter.java index d5d9068bc129..fa663141ea34 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/AndFilter.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/AndFilter.java @@ -22,6 +22,8 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; import com.google.common.base.Preconditions; +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import org.apache.druid.collections.bitmap.ImmutableBitmap; @@ -55,7 +57,7 @@ public class AndFilter implements BooleanFilter private final Set filters; - private Integer filtersHashCode; + private final Supplier filtersHashCode; @VisibleForTesting public AndFilter(List filters) @@ -67,6 +69,7 @@ public AndFilter(Set filters) { Preconditions.checkArgument(filters.size() > 0, "Can't construct empty AndFilter"); this.filters = filters; + this.filtersHashCode = Suppliers.memoize(() -> Objects.hash(getFilters())); } public static ImmutableBitmap getBitmapIndex( @@ -267,10 +270,6 @@ public boolean equals(Object o) @Override public int hashCode() { - if (filtersHashCode == null) { - filtersHashCode = Objects.hash(getFilters()); - } - - return filtersHashCode; + return filtersHashCode.get(); } } diff --git a/processing/src/main/java/org/apache/druid/segment/filter/InFilter.java b/processing/src/main/java/org/apache/druid/segment/filter/InFilter.java index 6472b6ca350e..8b8ce359d61e 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/InFilter.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/InFilter.java @@ -22,6 +22,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Predicate; import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableSet; import com.google.errorprone.annotations.Immutable; import it.unimi.dsi.fastutil.ints.IntIterable; @@ -75,7 +76,7 @@ public class InFilter implements Filter private final Supplier longPredicateSupplier; private final Supplier floatPredicateSupplier; private final Supplier doublePredicateSupplier; - private Integer valuesHashCode; + private final Supplier valuesHashCode; public InFilter( String dimension, @@ -85,7 +86,7 @@ public InFilter( Supplier doublePredicateSupplier, ExtractionFn extractionFn, FilterTuning filterTuning, - Integer valuesHashCode + Supplier valuesHashCode ) { this.dimension = dimension; @@ -95,7 +96,9 @@ public InFilter( this.longPredicateSupplier = longPredicateSupplier; this.floatPredicateSupplier = floatPredicateSupplier; this.doublePredicateSupplier = doublePredicateSupplier; - this.valuesHashCode = valuesHashCode; + this.valuesHashCode = valuesHashCode == null + ? Suppliers.memoize(() -> InDimFilter.computeValuesHashCode(values)) + : valuesHashCode; } @Override @@ -251,13 +254,8 @@ public boolean equals(Object o) return false; } InFilter inFilter = (InFilter) o; - - // make sure the hashCode is initialized for both filters - hashCode(); - inFilter.hashCode(); - return Objects.equals(dimension, inFilter.dimension) && - Objects.equals(valuesHashCode, inFilter.valuesHashCode) && + Objects.equals(valuesHashCode.get(), inFilter.valuesHashCode.get()) && Objects.equals(extractionFn, inFilter.extractionFn) && Objects.equals(filterTuning, inFilter.filterTuning); } @@ -265,10 +263,7 @@ public boolean equals(Object o) @Override public int hashCode() { - if (valuesHashCode == null) { - valuesHashCode = InDimFilter.computeValuesHashCode(values); - } - return Objects.hash(dimension, valuesHashCode, extractionFn, filterTuning); + return Objects.hash(dimension, valuesHashCode.get(), extractionFn, filterTuning); } @VisibleForTesting diff --git a/processing/src/main/java/org/apache/druid/segment/filter/OrFilter.java b/processing/src/main/java/org/apache/druid/segment/filter/OrFilter.java index 00d687f5b0c0..bd32cc2527c8 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/OrFilter.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/OrFilter.java @@ -22,6 +22,8 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; import com.google.common.base.Preconditions; +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import com.google.common.collect.Iterables; import org.apache.druid.collections.bitmap.ImmutableBitmap; import org.apache.druid.java.util.common.StringUtils; @@ -55,7 +57,7 @@ public class OrFilter implements BooleanFilter private final Set filters; - private Integer filtersHashCode; + private final Supplier filtersHashCode; @VisibleForTesting public OrFilter(List filters) @@ -68,6 +70,7 @@ public OrFilter(Set filters) Preconditions.checkArgument(filters.size() > 0, "Can't construct empty OrFilter (the universe does not exist)"); this.filters = filters; + this.filtersHashCode = Suppliers.memoize(() -> Objects.hash(getFilters())); } @Override @@ -252,10 +255,6 @@ public boolean equals(Object o) @Override public int hashCode() { - if (filtersHashCode == null) { - filtersHashCode = Objects.hash(getFilters()); - } - - return filtersHashCode; + return filtersHashCode.get(); } } diff --git a/processing/src/main/java/org/apache/druid/segment/join/filter/rewrite/JoinFilterPreAnalysisGroup.java b/processing/src/main/java/org/apache/druid/segment/join/filter/rewrite/JoinFilterPreAnalysisGroup.java index d43fd10264f3..72804718822c 100644 --- a/processing/src/main/java/org/apache/druid/segment/join/filter/rewrite/JoinFilterPreAnalysisGroup.java +++ b/processing/src/main/java/org/apache/druid/segment/join/filter/rewrite/JoinFilterPreAnalysisGroup.java @@ -77,15 +77,6 @@ public JoinFilterPreAnalysis computeJoinFilterPreAnalysisIfAbsent( VirtualColumns virtualColumns ) { - // Some filters have potentially expensive hash codes that are lazily computed and cached. - // We call hashCode() here in a synchronized block before we attempt to use the Filter in the analyses map, - // to ensure that the hashCode is only computed once per Filter since the Filter interface is not thread-safe. - synchronized (analyses) { - if (filter != null) { - filter.hashCode(); - } - } - JoinFilterPreAnalysisGroupKey key = new JoinFilterPreAnalysisGroupKey(filter, clauses, virtualColumns); return analyses.computeIfAbsent( key, diff --git a/processing/src/test/java/org/apache/druid/query/filter/InDimFilterTest.java b/processing/src/test/java/org/apache/druid/query/filter/InDimFilterTest.java index b9605adcc1c7..dfe9203c2703 100644 --- a/processing/src/test/java/org/apache/druid/query/filter/InDimFilterTest.java +++ b/processing/src/test/java/org/apache/druid/query/filter/InDimFilterTest.java @@ -20,6 +20,8 @@ package org.apache.druid.query.filter; import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; @@ -151,9 +153,9 @@ public void test_equals() { EqualsVerifier.forClass(InDimFilter.class) .usingGetClass() - .withNonnullFields("dimension", "values") + .withNonnullFields("dimension", "values", "valuesHashCode") .withIgnoredFields("values", "cacheKey", "longPredicateSupplier", "floatPredicateSupplier", "doublePredicateSupplier") + .withPrefabValues(Supplier.class, Suppliers.ofInstance(1), Suppliers.ofInstance(2)) .verify(); } - } diff --git a/processing/src/test/java/org/apache/druid/segment/filter/AndFilterTest.java b/processing/src/test/java/org/apache/druid/segment/filter/AndFilterTest.java index 1ff9bd5f878a..0efeb5800ba9 100644 --- a/processing/src/test/java/org/apache/druid/segment/filter/AndFilterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/filter/AndFilterTest.java @@ -20,6 +20,8 @@ package org.apache.druid.segment.filter; import com.google.common.base.Function; +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import nl.jqno.equalsverifier.EqualsVerifier; @@ -182,8 +184,13 @@ public void test_equals() { EqualsVerifier.forClass(AndFilter.class) .usingGetClass() - .withNonnullFields("filters") + .withNonnullFields("filters", "filtersHashCode") .withIgnoredFields("filters") + .withPrefabValues( + Supplier.class, + Suppliers.ofInstance(1), + Suppliers.ofInstance(2) + ) .verify(); } } diff --git a/processing/src/test/java/org/apache/druid/segment/filter/InFilterTest.java b/processing/src/test/java/org/apache/druid/segment/filter/InFilterTest.java index 0be6cf5b0c50..789be36e8e82 100644 --- a/processing/src/test/java/org/apache/druid/segment/filter/InFilterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/filter/InFilterTest.java @@ -20,6 +20,8 @@ package org.apache.druid.segment.filter; import com.google.common.base.Function; +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; @@ -383,8 +385,9 @@ public void test_equals() { EqualsVerifier.forClass(InFilter.class) .usingGetClass() - .withNonnullFields("dimension", "values") + .withNonnullFields("dimension", "values", "valuesHashCode") .withIgnoredFields("values", "longPredicateSupplier", "floatPredicateSupplier", "doublePredicateSupplier") + .withPrefabValues(Supplier.class, Suppliers.ofInstance(1), Suppliers.ofInstance(2)) .verify(); } diff --git a/processing/src/test/java/org/apache/druid/segment/filter/OrFilterTest.java b/processing/src/test/java/org/apache/druid/segment/filter/OrFilterTest.java index 844a0895e941..65f234778ff9 100644 --- a/processing/src/test/java/org/apache/druid/segment/filter/OrFilterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/filter/OrFilterTest.java @@ -19,14 +19,26 @@ package org.apache.druid.segment.filter; +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; import nl.jqno.equalsverifier.EqualsVerifier; +import org.apache.druid.testing.InitializedNullHandlingTest; import org.junit.Test; -public class OrFilterTest +public class OrFilterTest extends InitializedNullHandlingTest { @Test public void test_equals() { - EqualsVerifier.forClass(OrFilter.class).usingGetClass().withNonnullFields("filters").verify(); + EqualsVerifier.forClass(OrFilter.class) + .usingGetClass() + .withNonnullFields("filters", "filtersHashCode") + .withIgnoredFields("filters") + .withPrefabValues( + Supplier.class, + Suppliers.ofInstance(1), + Suppliers.ofInstance(2) + ) + .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 5df585bc2b20..85665d09bf4f 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 @@ -11483,7 +11483,7 @@ public void testTimeExtractWithTooFewArguments() throws Exception @Test @Parameters(source = QueryContextForJoinProvider.class) - public void testNestedGroupByOnInlineDataSourceWithFilterIsNotSupported(Map queryContext) throws Exception + public void testNestedGroupByOnInlineDataSourceWithFilter(Map queryContext) throws Exception { // Cannot vectorize due to virtual columns. cannotVectorize();