You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by mo...@apache.org on 2022/12/29 04:06:25 UTC
[doris] 03/03: [Bug] #14876 && #15225 have some bugs in rewrite or to in, revert them (#15420)
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 <45...@users.noreply.github.com>
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: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org