This is an automated email from the ASF dual-hosted git repository. morningman pushed a commit to branch branch-1.2-lts in repository https://gitbox.apache.org/repos/asf/doris.git
commit 7d8a94604e18c96b2b8af08cbfe22153d3ccb56d Author: Henry2SS <[email protected]> AuthorDate: Wed Dec 28 13:30:09 2022 +0800 [Bug] #14876 && #15225 have some bugs in rewrite or to in, revert them (#15420) --- .../java/org/apache/doris/analysis/Analyzer.java | 2 - .../java/org/apache/doris/qe/SessionVariable.java | 14 -- .../rewrite/CompactEqualsToInPredicateRule.java | 166 --------------------- .../doris/analysis/PartitionPruneTestBase.java | 2 + .../org/apache/doris/planner/QueryPlanTest.java | 5 +- .../CompactEqualsToInPredicateRuleTest.java | 116 -------------- .../performance_p0/redundant_conjuncts.groovy | 1 - 7 files changed, 4 insertions(+), 302 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/Analyzer.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/Analyzer.java index 9c5b2e0be1..946718b2c6 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/Analyzer.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/Analyzer.java @@ -46,7 +46,6 @@ import org.apache.doris.planner.PlanNode; import org.apache.doris.planner.RuntimeFilter; import org.apache.doris.qe.ConnectContext; import org.apache.doris.rewrite.BetweenToCompoundRule; -import org.apache.doris.rewrite.CompactEqualsToInPredicateRule; import org.apache.doris.rewrite.CompoundPredicateWriteRule; import org.apache.doris.rewrite.ExprRewriteRule; import org.apache.doris.rewrite.ExprRewriter; @@ -415,7 +414,6 @@ public class Analyzer { rules.add(RewriteEncryptKeyRule.INSTANCE); rules.add(RewriteInPredicateRule.INSTANCE); rules.add(RewriteAliasFunctionRule.INSTANCE); - rules.add(CompactEqualsToInPredicateRule.INSTANCE); List<ExprRewriteRule> onceRules = Lists.newArrayList(); onceRules.add(ExtractCommonFactorsRule.INSTANCE); onceRules.add(InferFiltersRule.INSTANCE); diff --git a/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java b/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java index e2e82e91d9..f0a556067f 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java +++ b/fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java @@ -196,9 +196,6 @@ public class SessionVariable implements Serializable, Writable { //percentage of EXEC_MEM_LIMIT public static final String BROADCAST_HASHTABLE_MEM_LIMIT_PERCENTAGE = "broadcast_hashtable_mem_limit_percentage"; - - public static final String COMPACT_EQUAL_TO_IN_PREDICATE_THRESHOLD = "compact_equal_to_in_predicate_threshold"; - public static final String NEREIDS_STAR_SCHEMA_SUPPORT = "nereids_star_schema_support"; public static final String NEREIDS_CBO_PENALTY_FACTOR = "nereids_cbo_penalty_factor"; @@ -547,9 +544,6 @@ public class SessionVariable implements Serializable, Writable { @VariableMgr.VarAttr(name = NEREIDS_STAR_SCHEMA_SUPPORT) private boolean nereidsStarSchemaSupport = true; - @VariableMgr.VarAttr(name = COMPACT_EQUAL_TO_IN_PREDICATE_THRESHOLD) - private int compactEqualToInPredicateThreshold = 2; - @VariableMgr.VarAttr(name = NEREIDS_CBO_PENALTY_FACTOR) private double nereidsCboPenaltyFactor = 0.7; @VariableMgr.VarAttr(name = ENABLE_NEREIDS_TRACE) @@ -667,14 +661,6 @@ public class SessionVariable implements Serializable, Writable { this.blockEncryptionMode = blockEncryptionMode; } - public void setCompactEqualToInPredicateThreshold(int threshold) { - this.compactEqualToInPredicateThreshold = threshold; - } - - public int getCompactEqualToInPredicateThreshold() { - return compactEqualToInPredicateThreshold; - } - public long getMaxExecMemByte() { return maxExecMemByte; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/rewrite/CompactEqualsToInPredicateRule.java b/fe/fe-core/src/main/java/org/apache/doris/rewrite/CompactEqualsToInPredicateRule.java deleted file mode 100644 index 2b8ad5ed2c..0000000000 --- a/fe/fe-core/src/main/java/org/apache/doris/rewrite/CompactEqualsToInPredicateRule.java +++ /dev/null @@ -1,166 +0,0 @@ -// 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.doris.rewrite; - -import org.apache.doris.analysis.Analyzer; -import org.apache.doris.analysis.BinaryPredicate; -import org.apache.doris.analysis.CompoundPredicate; -import org.apache.doris.analysis.CompoundPredicate.Operator; -import org.apache.doris.analysis.Expr; -import org.apache.doris.analysis.InPredicate; -import org.apache.doris.analysis.LiteralExpr; -import org.apache.doris.analysis.SlotRef; -import org.apache.doris.common.AnalysisException; -import org.apache.doris.common.Pair; -import org.apache.doris.qe.ConnectContext; -import org.apache.doris.rewrite.ExprRewriter.ClauseType; - -import com.google.common.collect.Lists; - -import java.util.ArrayList; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Map.Entry; -import java.util.Set; - -/* -a = 1 or a = 2 or a = 3 or a in (4, 5, 6) => a in (1, 2, 3, 4, 5, 6) - */ -public class CompactEqualsToInPredicateRule implements ExprRewriteRule { - public static CompactEqualsToInPredicateRule INSTANCE = new CompactEqualsToInPredicateRule(); - - @Override - public Expr apply(Expr expr, Analyzer analyzer, ClauseType clauseType) throws AnalysisException { - if (expr == null) { - return expr; - } - if (expr instanceof CompoundPredicate) { - CompoundPredicate comp = (CompoundPredicate) expr; - if (comp.getOp() == Operator.OR) { - Pair<Boolean, Expr> compactResult = compactEqualsToInPredicate(expr); - if (compactResult.first) { - return compactResult.second; - } - } - } - return expr; - } - - /* - expr in form of A or B or ... - */ - private Pair<Boolean, Expr> compactEqualsToInPredicate(Expr expr) { - int compactThreshold; - if (ConnectContext.get() == null) { - compactThreshold = 2; - } else { - compactThreshold = ConnectContext.get().getSessionVariable().getCompactEqualToInPredicateThreshold(); - } - boolean changed = false; - List<Expr> disConjuncts = getDisconjuncts(expr); - if (disConjuncts.size() < compactThreshold) { - return Pair.of(false, expr); - } - Map<SlotRef, Set<Expr>> equalMap = new HashMap<>(); - Map<SlotRef, InPredicate> inPredMap = new HashMap<>(); - Expr result = null; - for (Expr disConj : disConjuncts) { - if (disConj instanceof BinaryPredicate - && ((BinaryPredicate) disConj).getOp() == BinaryPredicate.Operator.EQ) { - BinaryPredicate binary = (BinaryPredicate) disConj; - if (binary.getChild(0) instanceof SlotRef - && binary.getChild(1) instanceof LiteralExpr) { - equalMap.computeIfAbsent((SlotRef) binary.getChild(0), k -> new HashSet<>()); - equalMap.get((SlotRef) binary.getChild(0)).add((LiteralExpr) binary.getChild(1)); - } else if (binary.getChild(0) instanceof LiteralExpr - && binary.getChild(1) instanceof SlotRef) { - equalMap.computeIfAbsent((SlotRef) binary.getChild(1), k -> new HashSet<>()); - equalMap.get((SlotRef) binary.getChild(1)).add((LiteralExpr) binary.getChild(0)); - } else { - result = addDisconjunct(result, disConj); - } - } else if (disConj instanceof InPredicate && !((InPredicate) disConj).isNotIn()) { - InPredicate in = (InPredicate) disConj; - Expr compareExpr = in.getChild(0); - if (compareExpr instanceof SlotRef) { - SlotRef slot = (SlotRef) compareExpr; - InPredicate val = inPredMap.get(slot); - if (val == null) { - inPredMap.put(slot, in); - } else { - val.getChildren().addAll(in.getListChildren()); - inPredMap.put(slot, val); - } - } - } else { - result = addDisconjunct(result, disConj); - } - } - - for (Entry<SlotRef, Set<Expr>> entry : equalMap.entrySet()) { - SlotRef slot = entry.getKey(); - InPredicate in = inPredMap.get(slot); - if (entry.getValue().size() >= compactThreshold || in != null) { - if (in == null) { - in = new InPredicate(entry.getKey(), Lists.newArrayList(entry.getValue()), false); - inPredMap.put(slot, in); - } else { - in.getChildren().addAll(Lists.newArrayList(entry.getValue())); - } - changed = true; - } else { - for (Expr right : entry.getValue()) { - result = addDisconjunct(result, - new BinaryPredicate(BinaryPredicate.Operator.EQ, - entry.getKey(), - right)); - } - } - } - for (InPredicate in : inPredMap.values()) { - result = addDisconjunct(result, in); - } - return Pair.of(changed, result); - } - - private Expr addDisconjunct(Expr result, Expr conj) { - if (result == null) { - return conj; - } else { - return new CompoundPredicate(Operator.OR, result, conj); - } - } - - private List<Expr> getDisconjuncts(Expr expr) { - List<Expr> result = new ArrayList<>(); - if (expr instanceof CompoundPredicate) { - CompoundPredicate comp = ((CompoundPredicate) expr); - if (comp.getOp() == Operator.OR) { - result.addAll(getDisconjuncts(comp.getChild(0))); - result.addAll(getDisconjuncts(comp.getChild(1))); - } else { - result.add(expr); - } - } else { - result.add(expr); - } - return result; - } -} diff --git a/fe/fe-core/src/test/java/org/apache/doris/analysis/PartitionPruneTestBase.java b/fe/fe-core/src/test/java/org/apache/doris/analysis/PartitionPruneTestBase.java index 7ff8e2fda1..95c4790bea 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/analysis/PartitionPruneTestBase.java +++ b/fe/fe-core/src/test/java/org/apache/doris/analysis/PartitionPruneTestBase.java @@ -29,6 +29,8 @@ public abstract class PartitionPruneTestBase extends TestWithFeService { protected void doTest() throws Exception { for (RangePartitionPruneTest.TestCase testCase : cases) { + connectContext.getSessionVariable().partitionPruneAlgorithmVersion = 1; + assertExplainContains(1, testCase.sql, testCase.v1Result); connectContext.getSessionVariable().partitionPruneAlgorithmVersion = 2; assertExplainContains(2, testCase.sql, testCase.v2Result); } diff --git a/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java b/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java index 2a4e429ebd..8d1ad8b78c 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java @@ -2233,11 +2233,10 @@ public class QueryPlanTest extends TestWithFeService { sql = "SELECT * from test1 where (query_time = 1 or query_time = 2 or scan_bytes = 2) and scan_bytes in (2, 3)"; explainString = UtFrameUtils.getSQLPlanOrErrorMsg(connectContext, "EXPLAIN " + sql); - Assert.assertTrue(explainString.contains("PREDICATES: (`query_time` IN (1, 2) OR `scan_bytes` = 2), `scan_bytes` IN (2, 3)")); + Assert.assertTrue(explainString.contains("PREDICATES: (`query_time` = 1 OR `query_time` = 2 OR `scan_bytes` = 2), `scan_bytes` IN (2, 3)")); sql = "SELECT * from test1 where (query_time = 1 or query_time = 2) and (scan_bytes = 2 or scan_bytes = 3)"; explainString = UtFrameUtils.getSQLPlanOrErrorMsg(connectContext, "EXPLAIN " + sql); - Assert.assertTrue(explainString.contains("PREDICATES: `query_time` IN (1, 2), `scan_bytes` IN (2, 3)") - || explainString.contains("PREDICATES: `query_time` IN (1, 2), `scan_bytes` IN (3, 2)")); + Assert.assertTrue(explainString.contains("PREDICATES: `query_time` IN (1, 2), `scan_bytes` IN (2, 3)")); } } diff --git a/fe/fe-core/src/test/java/org/apache/doris/rewrite/CompactEqualsToInPredicateRuleTest.java b/fe/fe-core/src/test/java/org/apache/doris/rewrite/CompactEqualsToInPredicateRuleTest.java deleted file mode 100644 index baa694ea42..0000000000 --- a/fe/fe-core/src/test/java/org/apache/doris/rewrite/CompactEqualsToInPredicateRuleTest.java +++ /dev/null @@ -1,116 +0,0 @@ -// 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.doris.rewrite; - -import org.apache.doris.analysis.BinaryPredicate; -import org.apache.doris.analysis.CompoundPredicate; -import org.apache.doris.analysis.CompoundPredicate.Operator; -import org.apache.doris.analysis.InPredicate; -import org.apache.doris.analysis.IntLiteral; -import org.apache.doris.analysis.SlotRef; -import org.apache.doris.analysis.TableName; -import org.apache.doris.common.Pair; -import org.apache.doris.common.jmockit.Deencapsulation; -import org.apache.doris.datasource.InternalCatalog; - -import com.google.common.collect.Lists; -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.Test; - -import java.util.HashSet; - -public class CompactEqualsToInPredicateRuleTest { - private static final String internalCtl = InternalCatalog.INTERNAL_CATALOG_NAME; - - //a=1 or b=2 or a=3 or b=4 - //=> a in (1, 2) or b in (3, 4) - @Test - public void testCompactEquals() { - SlotRef a = new SlotRef(new TableName(internalCtl, "db1", "tb1"), "a"); - SlotRef b = new SlotRef(new TableName(internalCtl, "db1", "tb1"), "b"); - IntLiteral i1 = new IntLiteral(1); - IntLiteral i2 = new IntLiteral(2); - IntLiteral i3 = new IntLiteral(3); - IntLiteral i4 = new IntLiteral(4); - BinaryPredicate aeq1 = new BinaryPredicate(BinaryPredicate.Operator.EQ, i1, a); - BinaryPredicate aeq3 = new BinaryPredicate(BinaryPredicate.Operator.EQ, a, i3); - BinaryPredicate beq2 = new BinaryPredicate(BinaryPredicate.Operator.EQ, b, i2); - BinaryPredicate beq4 = new BinaryPredicate(BinaryPredicate.Operator.EQ, b, i4); - CompoundPredicate or1 = new CompoundPredicate(Operator.OR, aeq1, beq2); - CompoundPredicate or2 = new CompoundPredicate(Operator.OR, or1, aeq3); - CompoundPredicate or3 = new CompoundPredicate(Operator.OR, or2, beq4); - CompactEqualsToInPredicateRule rule = new CompactEqualsToInPredicateRule(); - Pair result = Deencapsulation.invoke(rule, - "compactEqualsToInPredicate", or3); - Assertions.assertEquals(true, result.first); - Assertions.assertTrue(result.second instanceof CompoundPredicate); - CompoundPredicate or = (CompoundPredicate) result.second; - Assertions.assertEquals(Operator.OR, or.getOp()); - InPredicate in1 = (InPredicate) or.getChild(0); - InPredicate in2 = (InPredicate) or.getChild(1); - SlotRef s1 = (SlotRef) in1.getChildren().get(0); - InPredicate tmp; - if (s1.getColumnName().equals("b")) { - tmp = in1; - in1 = in2; - in2 = tmp; - } - Assertions.assertEquals(in1.getChild(0), a); - Assertions.assertEquals(in2.getChild(0), b); - - HashSet<IntLiteral> seta = new HashSet<>(); - seta.add(i1); - seta.add(i3); - HashSet<IntLiteral> setb = new HashSet<>(); - setb.add(i2); - setb.add(i4); - - Assertions.assertTrue(seta.contains(in1.getChild(1))); - Assertions.assertTrue(seta.contains(in1.getChild(2))); - - Assertions.assertTrue(setb.contains(in2.getChild(1))); - Assertions.assertTrue(setb.contains(in2.getChild(2))); - } - - //a=1 or a in (3, 2) => a in (1, 2, 3) - @Test - public void testCompactEqualsAndIn() { - SlotRef a = new SlotRef(new TableName(internalCtl, "db1", "tb1"), "a"); - IntLiteral i1 = new IntLiteral(1); - IntLiteral i2 = new IntLiteral(2); - IntLiteral i3 = new IntLiteral(3); - BinaryPredicate aeq1 = new BinaryPredicate(BinaryPredicate.Operator.EQ, i1, a); - InPredicate ain23 = new InPredicate(a, Lists.newArrayList(i2, i3), false); - CompoundPredicate or1 = new CompoundPredicate(Operator.OR, aeq1, ain23); - CompactEqualsToInPredicateRule rule = new CompactEqualsToInPredicateRule(); - Pair result = Deencapsulation.invoke(rule, - "compactEqualsToInPredicate", or1); - Assertions.assertEquals(true, result.first); - Assertions.assertTrue(result.second instanceof InPredicate); - InPredicate in123 = (InPredicate) result.second; - Assertions.assertEquals(in123.getChild(0), a); - HashSet<IntLiteral> seta = new HashSet<>(); - seta.add(i1); - seta.add(i2); - seta.add(i3); - - Assertions.assertTrue(seta.contains(in123.getChild(1))); - Assertions.assertTrue(seta.contains(in123.getChild(2))); - Assertions.assertTrue(seta.contains(in123.getChild(3))); - } -} diff --git a/regression-test/suites/performance_p0/redundant_conjuncts.groovy b/regression-test/suites/performance_p0/redundant_conjuncts.groovy index 3fbe2c9237..14624a8049 100644 --- a/regression-test/suites/performance_p0/redundant_conjuncts.groovy +++ b/regression-test/suites/performance_p0/redundant_conjuncts.groovy @@ -39,7 +39,6 @@ suite("redundant_conjuncts") { EXPLAIN SELECT v1 FROM redundant_conjuncts WHERE k1 = 1 AND k1 = 1; """ - sql "set COMPACT_EQUAL_TO_IN_PREDICATE_THRESHOLD = 100" qt_redundant_conjuncts_gnerated_by_extract_common_filter """ EXPLAIN SELECT v1 FROM redundant_conjuncts WHERE k1 = 1 OR k1 = 2; """ --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
