This is an automated email from the ASF dual-hosted git repository.
huajianlan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 69bfbae856 [enhancement](nereids) Normalize expressions before
performing plan rewriting (#11299)
69bfbae856 is described below
commit 69bfbae856ef2219ac31c01b4a49f40680293419
Author: Adonis Ling <adonis0...@gmail.com>
AuthorDate: Mon Aug 1 17:15:04 2022 +0800
[enhancement](nereids) Normalize expressions before performing plan
rewriting (#11299)
Rules for normalizing expressions should be applied once before do some
extra expression transforms.
Normalization rules include:
1. NormalizeBinaryPredicatesRule
2. BetweenToCompoundRule
3. SimplifyNotExprRule
---
.../org/apache/doris/nereids/NereidsPlanner.java | 2 ++
.../batch/NormalizeExpressionRulesJob.java} | 33 ++++++++++++----------
.../java/org/apache/doris/nereids/memo/Memo.java | 2 +-
...ionOfPlan.java => ExpressionNormalization.java} | 8 ++++--
...sionOfPlan.java => ExpressionOptimization.java} | 6 ++--
...onOfPlanRewrite.java => ExpressionRewrite.java} | 4 +--
.../expression/rewrite/ExpressionRuleExecutor.java | 15 ----------
.../rewrite/logical/PushPredicateThroughJoin.java | 6 ++--
.../apache/doris/nereids/trees/TernaryNode.java | 2 +-
.../trees/expressions/BinaryExpression.java | 11 --------
.../trees/expressions/TernaryExpression.java | 13 ---------
.../nereids/trees/expressions/UnaryExpression.java | 5 ----
.../rewrite/logical/PushDownPredicateTest.java | 4 ++-
13 files changed, 36 insertions(+), 75 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/NereidsPlanner.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/NereidsPlanner.java
index 3ccb0615d1..57f4e267ee 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/NereidsPlanner.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/NereidsPlanner.java
@@ -27,6 +27,7 @@ import
org.apache.doris.nereids.glue.translator.PhysicalPlanTranslator;
import org.apache.doris.nereids.glue.translator.PlanTranslatorContext;
import org.apache.doris.nereids.jobs.batch.DisassembleRulesJob;
import org.apache.doris.nereids.jobs.batch.JoinReorderRulesJob;
+import org.apache.doris.nereids.jobs.batch.NormalizeExpressionRulesJob;
import org.apache.doris.nereids.jobs.batch.OptimizeRulesJob;
import org.apache.doris.nereids.jobs.batch.PredicatePushDownRulesJob;
import org.apache.doris.nereids.jobs.cascades.DeriveStatsJob;
@@ -120,6 +121,7 @@ public class NereidsPlanner extends Planner {
* Logical plan rewrite based on a series of heuristic rules.
*/
private void rewrite() {
+ new NormalizeExpressionRulesJob(plannerContext).execute();
new JoinReorderRulesJob(plannerContext).execute();
new PredicatePushDownRulesJob(plannerContext).execute();
new DisassembleRulesJob(plannerContext).execute();
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/TernaryExpression.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/batch/NormalizeExpressionRulesJob.java
similarity index 54%
copy from
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/TernaryExpression.java
copy to
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/batch/NormalizeExpressionRulesJob.java
index c5a86c5541..0e7c1b9549 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/TernaryExpression.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/batch/NormalizeExpressionRulesJob.java
@@ -15,25 +15,28 @@
// specific language governing permissions and limitations
// under the License.
-package org.apache.doris.nereids.trees.expressions;
+package org.apache.doris.nereids.jobs.batch;
-import org.apache.doris.nereids.trees.TernaryNode;
+import org.apache.doris.nereids.PlannerContext;
+import
org.apache.doris.nereids.rules.expression.rewrite.ExpressionNormalization;
+
+import com.google.common.collect.ImmutableList;
/**
- * Interface for all expression that have three children.
+ * Apply rules to normalize expressions.
*/
-public interface TernaryExpression extends TernaryNode<Expression, Expression,
Expression, Expression> {
-
-
- default Expression first() {
- return child(0);
- }
-
- default Expression second() {
- return child(1);
- }
+public class NormalizeExpressionRulesJob extends BatchRulesJob {
- default Expression third() {
- return child(2);
+ /**
+ * Constructor.
+ * @param plannerContext context for applying rules.
+ */
+ public NormalizeExpressionRulesJob(PlannerContext plannerContext) {
+ super(plannerContext);
+ rulesJob.addAll(ImmutableList.of(
+ topDownBatch(ImmutableList.of(
+ new ExpressionNormalization()
+ ))
+ ));
}
}
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/memo/Memo.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/memo/Memo.java
index 16d02e8fa9..6f13bb7fa2 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/memo/Memo.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/memo/Memo.java
@@ -247,7 +247,7 @@ public class Memo {
private Plan replaceChildrenToGroupPlan(Plan plan, List<Group>
childrenGroups) {
List<Plan> groupPlanChildren = childrenGroups.stream()
- .map(group -> new GroupPlan(group))
+ .map(GroupPlan::new)
.collect(ImmutableList.toImmutableList());
LogicalProperties logicalProperties = plan.getLogicalProperties();
return plan.withChildren(groupPlanChildren)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/NormalizeExpressionOfPlan.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/ExpressionNormalization.java
similarity index 84%
rename from
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/NormalizeExpressionOfPlan.java
rename to
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/ExpressionNormalization.java
index 44970483f9..3cd9357325 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/NormalizeExpressionOfPlan.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/ExpressionNormalization.java
@@ -19,6 +19,7 @@ package org.apache.doris.nereids.rules.expression.rewrite;
import
org.apache.doris.nereids.rules.expression.rewrite.rules.BetweenToCompoundRule;
import
org.apache.doris.nereids.rules.expression.rewrite.rules.NormalizeBinaryPredicatesRule;
+import
org.apache.doris.nereids.rules.expression.rewrite.rules.SimplifyNotExprRule;
import com.google.common.collect.ImmutableList;
@@ -27,15 +28,16 @@ import java.util.List;
/**
* normalize expression of plan rule set.
*/
-public class NormalizeExpressionOfPlan extends ExpressionOfPlanRewrite {
+public class ExpressionNormalization extends ExpressionRewrite {
public static final List<ExpressionRewriteRule> NORMALIZE_REWRITE_RULES =
ImmutableList.of(
NormalizeBinaryPredicatesRule.INSTANCE,
- BetweenToCompoundRule.INSTANCE
+ BetweenToCompoundRule.INSTANCE,
+ SimplifyNotExprRule.INSTANCE
);
private static final ExpressionRuleExecutor EXECUTOR = new
ExpressionRuleExecutor(NORMALIZE_REWRITE_RULES);
- public NormalizeExpressionOfPlan() {
+ public ExpressionNormalization() {
super(EXECUTOR);
}
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/OptimizeExpressionOfPlan.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/ExpressionOptimization.java
similarity index 86%
rename from
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/OptimizeExpressionOfPlan.java
rename to
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/ExpressionOptimization.java
index 44bb3ed06e..be0e7f0763 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/OptimizeExpressionOfPlan.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/ExpressionOptimization.java
@@ -19,7 +19,6 @@ package org.apache.doris.nereids.rules.expression.rewrite;
import
org.apache.doris.nereids.rules.expression.rewrite.rules.DistinctPredicatesRule;
import
org.apache.doris.nereids.rules.expression.rewrite.rules.ExtractCommonFactorRule;
-import
org.apache.doris.nereids.rules.expression.rewrite.rules.SimplifyNotExprRule;
import com.google.common.collect.ImmutableList;
@@ -28,14 +27,13 @@ import java.util.List;
/**
* optimize expression of plan rule set.
*/
-public class OptimizeExpressionOfPlan extends ExpressionOfPlanRewrite {
+public class ExpressionOptimization extends ExpressionRewrite {
public static final List<ExpressionRewriteRule> OPTIMIZE_REWRITE_RULES =
ImmutableList.of(
- SimplifyNotExprRule.INSTANCE,
ExtractCommonFactorRule.INSTANCE,
DistinctPredicatesRule.INSTANCE);
private static final ExpressionRuleExecutor EXECUTOR = new
ExpressionRuleExecutor(OPTIMIZE_REWRITE_RULES);
- public OptimizeExpressionOfPlan() {
+ public ExpressionOptimization() {
super(EXECUTOR);
}
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/ExpressionOfPlanRewrite.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/ExpressionRewrite.java
similarity index 97%
rename from
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/ExpressionOfPlanRewrite.java
rename to
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/ExpressionRewrite.java
index aaf1cc60ff..b285eaa2fa 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/ExpressionOfPlanRewrite.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/ExpressionRewrite.java
@@ -38,10 +38,10 @@ import java.util.stream.Collectors;
/**
* expression of plan rewrite rule.
*/
-public class ExpressionOfPlanRewrite implements RewriteRuleFactory {
+public class ExpressionRewrite implements RewriteRuleFactory {
private final ExpressionRuleExecutor rewriter;
- public ExpressionOfPlanRewrite(ExpressionRuleExecutor rewriter) {
+ public ExpressionRewrite(ExpressionRuleExecutor rewriter) {
this.rewriter = Objects.requireNonNull(rewriter, "rewriter is null");
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/ExpressionRuleExecutor.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/ExpressionRuleExecutor.java
index d2e66a2c3e..1e38dcd2bd 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/ExpressionRuleExecutor.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/ExpressionRuleExecutor.java
@@ -17,12 +17,8 @@
package org.apache.doris.nereids.rules.expression.rewrite;
-import
org.apache.doris.nereids.rules.expression.rewrite.rules.BetweenToCompoundRule;
-import
org.apache.doris.nereids.rules.expression.rewrite.rules.NormalizeBinaryPredicatesRule;
-import
org.apache.doris.nereids.rules.expression.rewrite.rules.SimplifyNotExprRule;
import org.apache.doris.nereids.trees.expressions.Expression;
-import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import java.util.List;
@@ -33,20 +29,9 @@ import java.util.stream.Collectors;
*/
public class ExpressionRuleExecutor {
- public static final List<ExpressionRewriteRule> REWRITE_RULES =
ImmutableList.of(
- new BetweenToCompoundRule(),
- new SimplifyNotExprRule(),
- new NormalizeBinaryPredicatesRule()
- );
-
private final ExpressionRewriteContext ctx;
private final List<ExpressionRewriteRule> rules;
- public ExpressionRuleExecutor() {
- this.rules = REWRITE_RULES;
- this.ctx = new ExpressionRewriteContext();
- }
-
public ExpressionRuleExecutor(List<ExpressionRewriteRule> rules) {
this.rules = rules;
this.ctx = new ExpressionRewriteContext();
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/logical/PushPredicateThroughJoin.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/logical/PushPredicateThroughJoin.java
index 0b79ea062c..5fff975229 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/logical/PushPredicateThroughJoin.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/logical/PushPredicateThroughJoin.java
@@ -19,7 +19,6 @@ package org.apache.doris.nereids.rules.rewrite.logical;
import org.apache.doris.nereids.rules.Rule;
import org.apache.doris.nereids.rules.RuleType;
-import
org.apache.doris.nereids.rules.expression.rewrite.ExpressionRuleExecutor;
import org.apache.doris.nereids.rules.rewrite.OneRewriteRuleFactory;
import org.apache.doris.nereids.trees.expressions.BooleanLiteral;
import org.apache.doris.nereids.trees.expressions.ComparisonPredicate;
@@ -122,15 +121,14 @@ public class PushPredicateThroughJoin extends
OneRewriteRuleFactory {
Expression left = ExpressionUtils.and(leftPredicates);
Expression right = ExpressionUtils.and(rightPredicates);
//todo expr should optimize again using expr rewrite
- ExpressionRuleExecutor exprRewriter = new ExpressionRuleExecutor();
Plan leftPlan = joinPlan.left();
Plan rightPlan = joinPlan.right();
if (!left.equals(BooleanLiteral.TRUE)) {
- leftPlan = new LogicalFilter(exprRewriter.rewrite(left), leftPlan);
+ leftPlan = new LogicalFilter(left, leftPlan);
}
if (!right.equals(BooleanLiteral.TRUE)) {
- rightPlan = new LogicalFilter(exprRewriter.rewrite(right),
rightPlan);
+ rightPlan = new LogicalFilter(right, rightPlan);
}
return new LogicalJoin<>(joinPlan.getJoinType(),
Optional.of(joinConditions), leftPlan, rightPlan);
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/TernaryNode.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/TernaryNode.java
index d598d604cd..d1c74a8204 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/TernaryNode.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/TernaryNode.java
@@ -18,7 +18,7 @@
package org.apache.doris.nereids.trees;
/**
- * interface for all tree node that have two children.
+ * interface for all tree node that have three children.
*/
public interface TernaryNode<
NODE_TYPE extends TreeNode<NODE_TYPE>,
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/BinaryExpression.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/BinaryExpression.java
index ccf9ea9b61..8965e36b6f 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/BinaryExpression.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/BinaryExpression.java
@@ -23,15 +23,4 @@ import org.apache.doris.nereids.trees.BinaryNode;
* Interface for all expression that have two children.
*/
public interface BinaryExpression extends BinaryNode<Expression, Expression,
Expression> {
-
- @Override
- default Expression left() {
- return child(0);
- }
-
- @Override
- default Expression right() {
- return child(1);
- }
-
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/TernaryExpression.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/TernaryExpression.java
index c5a86c5541..bea2644e54 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/TernaryExpression.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/TernaryExpression.java
@@ -23,17 +23,4 @@ import org.apache.doris.nereids.trees.TernaryNode;
* Interface for all expression that have three children.
*/
public interface TernaryExpression extends TernaryNode<Expression, Expression,
Expression, Expression> {
-
-
- default Expression first() {
- return child(0);
- }
-
- default Expression second() {
- return child(1);
- }
-
- default Expression third() {
- return child(2);
- }
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/UnaryExpression.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/UnaryExpression.java
index eca639d13c..ba37beee60 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/UnaryExpression.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/UnaryExpression.java
@@ -23,9 +23,4 @@ import org.apache.doris.nereids.trees.UnaryNode;
* Abstract class for all expression that have one child.
*/
public interface UnaryExpression extends UnaryNode<Expression, Expression> {
-
- @Override
- default Expression child() {
- return child(0);
- }
}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/logical/PushDownPredicateTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/logical/PushDownPredicateTest.java
index 7a81d1f9a2..c5525add55 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/logical/PushDownPredicateTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/logical/PushDownPredicateTest.java
@@ -23,6 +23,7 @@ import org.apache.doris.catalog.Table;
import org.apache.doris.catalog.Type;
import org.apache.doris.nereids.memo.Group;
import org.apache.doris.nereids.memo.Memo;
+import
org.apache.doris.nereids.rules.expression.rewrite.ExpressionNormalization;
import org.apache.doris.nereids.trees.expressions.Add;
import org.apache.doris.nereids.trees.expressions.And;
import org.apache.doris.nereids.trees.expressions.Between;
@@ -242,6 +243,7 @@ public class PushDownPredicateTest {
}
private Memo rewrite(Plan plan) {
- return PlanRewriter.topDownRewriteMemo(plan, new ConnectContext(), new
PushPredicateThroughJoin());
+ Plan normalizedPlan = PlanRewriter.topDownRewrite(plan, new
ConnectContext(), new ExpressionNormalization());
+ return PlanRewriter.topDownRewriteMemo(normalizedPlan, new
ConnectContext(), new PushPredicateThroughJoin());
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
