From 69bfbae856ef2219ac31c01b4a49f40680293419 Mon Sep 17 00:00:00 2001 From: Adonis Ling Date: Mon, 1 Aug 2022 17:15:04 +0800 Subject: [PATCH] [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 --- .../apache/doris/nereids/NereidsPlanner.java | 2 + .../batch/NormalizeExpressionRulesJob.java | 42 +++++++++++++++++++ .../org/apache/doris/nereids/memo/Memo.java | 2 +- ...Plan.java => ExpressionNormalization.java} | 8 ++-- ...fPlan.java => ExpressionOptimization.java} | 6 +-- ...lanRewrite.java => ExpressionRewrite.java} | 4 +- .../rewrite/ExpressionRuleExecutor.java | 15 ------- .../logical/PushPredicateThroughJoin.java | 6 +-- .../doris/nereids/trees/TernaryNode.java | 2 +- .../trees/expressions/BinaryExpression.java | 11 ----- .../trees/expressions/TernaryExpression.java | 13 ------ .../trees/expressions/UnaryExpression.java | 5 --- .../logical/PushDownPredicateTest.java | 4 +- 13 files changed, 60 insertions(+), 60 deletions(-) create mode 100644 fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/batch/NormalizeExpressionRulesJob.java rename fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/{NormalizeExpressionOfPlan.java => ExpressionNormalization.java} (84%) rename fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/{OptimizeExpressionOfPlan.java => ExpressionOptimization.java} (86%) rename fe/fe-core/src/main/java/org/apache/doris/nereids/rules/expression/rewrite/{ExpressionOfPlanRewrite.java => ExpressionRewrite.java} (97%) 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/jobs/batch/NormalizeExpressionRulesJob.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/batch/NormalizeExpressionRulesJob.java new file mode 100644 index 0000000000..0e7c1b9549 --- /dev/null +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/batch/NormalizeExpressionRulesJob.java @@ -0,0 +1,42 @@ +// 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.nereids.jobs.batch; + +import org.apache.doris.nereids.PlannerContext; +import org.apache.doris.nereids.rules.expression.rewrite.ExpressionNormalization; + +import com.google.common.collect.ImmutableList; + +/** + * Apply rules to normalize expressions. + */ +public class NormalizeExpressionRulesJob extends BatchRulesJob { + + /** + * 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 childrenGroups) { List 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 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 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 REWRITE_RULES = ImmutableList.of( - new BetweenToCompoundRule(), - new SimplifyNotExprRule(), - new NormalizeBinaryPredicatesRule() - ); - private final ExpressionRewriteContext ctx; private final List rules; - public ExpressionRuleExecutor() { - this.rules = REWRITE_RULES; - this.ctx = new ExpressionRewriteContext(); - } - public ExpressionRuleExecutor(List 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, 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 { - - @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 { - - - 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 { - - @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()); } }