This is an automated email from the ASF dual-hosted git repository. morrysnow 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 f2ed1bce1a [fix](nereids)change PushdownFilterThroughProject post processor from bottom up to top down rewrite (#21125) f2ed1bce1a is described below commit f2ed1bce1a3b98918f9af0db0d3f9d992421a5d0 Author: starocean999 <40539150+starocean...@users.noreply.github.com> AuthorDate: Mon Jun 26 15:34:41 2023 +0800 [fix](nereids)change PushdownFilterThroughProject post processor from bottom up to top down rewrite (#21125) 1. pass physicalProperties in withChildren function 2. use top down traverse in PushdownFilterThroughProject post processor --- .../post/PushdownFilterThroughProject.java | 20 ++-- .../plans/physical/PhysicalAssertNumRows.java | 3 +- .../trees/plans/physical/PhysicalCTEAnchor.java | 3 +- .../trees/plans/physical/PhysicalFilter.java | 8 +- .../trees/plans/physical/PhysicalGenerate.java | 4 +- .../trees/plans/physical/PhysicalLimit.java | 3 +- .../plans/physical/PhysicalOlapTableSink.java | 5 +- .../plans/physical/PhysicalPartitionTopN.java | 5 +- .../trees/plans/physical/PhysicalProject.java | 8 +- .../trees/plans/physical/PhysicalRepeat.java | 3 +- .../nereids/trees/plans/physical/PhysicalTopN.java | 3 +- .../trees/plans/physical/PhysicalWindow.java | 4 +- .../PushdownFilterThroughProjectTest.java | 112 +++++++++++++++++++++ 13 files changed, 155 insertions(+), 26 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/PushdownFilterThroughProject.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/PushdownFilterThroughProject.java index 350b053f0e..3eba798acb 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/PushdownFilterThroughProject.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/PushdownFilterThroughProject.java @@ -30,17 +30,15 @@ public class PushdownFilterThroughProject extends PlanPostProcessor { @Override public Plan visitPhysicalFilter(PhysicalFilter<? extends Plan> filter, CascadesContext context) { Plan child = filter.child(); - Plan newChild = child.accept(this, context); - if (!(newChild instanceof PhysicalProject)) { - return filter; + if (!(child instanceof PhysicalProject)) { + return filter.withChildren(child.accept(this, context)); } - PhysicalProject<? extends Plan> project = (PhysicalProject<? extends Plan>) newChild; - return project.withChildren( - new PhysicalFilter<>( - ExpressionUtils.replace(filter.getConjuncts(), project.getAliasToProducer()), - filter.getLogicalProperties(), - project.child() - ) - ); + + PhysicalProject<? extends Plan> project = (PhysicalProject<? extends Plan>) child; + PhysicalFilter<? extends Plan> newFilter = filter.withConjunctsAndChild( + ExpressionUtils.replace(filter.getConjuncts(), project.getAliasToProducer()), + project.child()); + + return project.withChildren(newFilter.accept(this, context)); } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalAssertNumRows.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalAssertNumRows.java index 0869c75d7d..de3ded2977 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalAssertNumRows.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalAssertNumRows.java @@ -105,7 +105,8 @@ public class PhysicalAssertNumRows<CHILD_TYPE extends Plan> extends PhysicalUnar @Override public PhysicalAssertNumRows<Plan> withChildren(List<Plan> children) { Preconditions.checkArgument(children.size() == 1); - return new PhysicalAssertNumRows<>(assertNumRowsElement, getLogicalProperties(), children.get(0)); + return new PhysicalAssertNumRows<>(assertNumRowsElement, groupExpression, + getLogicalProperties(), physicalProperties, statistics, children.get(0)); } @Override diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalCTEAnchor.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalCTEAnchor.java index 8ffb9de845..4d5dd8b16e 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalCTEAnchor.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalCTEAnchor.java @@ -107,7 +107,8 @@ public class PhysicalCTEAnchor< @Override public PhysicalCTEAnchor<Plan, Plan> withChildren(List<Plan> children) { Preconditions.checkArgument(children.size() == 2); - return new PhysicalCTEAnchor<>(cteId, getLogicalProperties(), children.get(0), children.get(1)); + return new PhysicalCTEAnchor<>(cteId, groupExpression, getLogicalProperties(), physicalProperties, + statistics, children.get(0), children.get(1)); } @Override diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalFilter.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalFilter.java index 866fdda4c6..899a35ff20 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalFilter.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalFilter.java @@ -104,7 +104,8 @@ public class PhysicalFilter<CHILD_TYPE extends Plan> extends PhysicalUnary<CHILD @Override public PhysicalFilter<Plan> withChildren(List<Plan> children) { Preconditions.checkArgument(children.size() == 1); - return new PhysicalFilter<>(conjuncts, getLogicalProperties(), children.get(0)); + return new PhysicalFilter<>(conjuncts, groupExpression, getLogicalProperties(), physicalProperties, + statistics, children.get(0)); } @Override @@ -124,6 +125,11 @@ public class PhysicalFilter<CHILD_TYPE extends Plan> extends PhysicalUnary<CHILD statistics, child()); } + public PhysicalFilter<Plan> withConjunctsAndChild(Set<Expression> conjuncts, Plan child) { + return new PhysicalFilter<>(conjuncts, groupExpression, getLogicalProperties(), physicalProperties, + statistics, child); + } + @Override public String shapeInfo() { StringBuilder builder = new StringBuilder(); diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalGenerate.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalGenerate.java index 862deb96d2..ec9887fb07 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalGenerate.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalGenerate.java @@ -127,8 +127,8 @@ public class PhysicalGenerate<CHILD_TYPE extends Plan> extends PhysicalUnary<CHI @Override public PhysicalGenerate<Plan> withChildren(List<Plan> children) { Preconditions.checkArgument(children.size() == 1); - return new PhysicalGenerate<>(generators, generatorOutput, - getLogicalProperties(), children.get(0)); + return new PhysicalGenerate<>(generators, generatorOutput, groupExpression, + getLogicalProperties(), physicalProperties, statistics, children.get(0)); } @Override diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalLimit.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalLimit.java index d98e394d82..8d5fb2a6ee 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalLimit.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalLimit.java @@ -103,7 +103,8 @@ public class PhysicalLimit<CHILD_TYPE extends Plan> extends PhysicalUnary<CHILD_ @Override public Plan withChildren(List<Plan> children) { Preconditions.checkArgument(children.size() == 1); - return new PhysicalLimit<>(limit, offset, phase, getLogicalProperties(), children.get(0)); + return new PhysicalLimit<>(limit, offset, phase, groupExpression, getLogicalProperties(), + physicalProperties, statistics, children.get(0)); } @Override diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalOlapTableSink.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalOlapTableSink.java index 21d36dda2a..8e37673497 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalOlapTableSink.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalOlapTableSink.java @@ -113,8 +113,9 @@ public class PhysicalOlapTableSink<CHILD_TYPE extends Plan> extends PhysicalUnar @Override public Plan withChildren(List<Plan> children) { Preconditions.checkArgument(children.size() == 1, "PhysicalOlapTableSink only accepts one child"); - return new PhysicalOlapTableSink<>(database, targetTable, partitionIds, cols, singleReplicaLoad, - getLogicalProperties(), children.get(0)); + return new PhysicalOlapTableSink<>(database, targetTable, partitionIds, cols, + singleReplicaLoad, groupExpression, getLogicalProperties(), physicalProperties, + statistics, children.get(0)); } @Override diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalPartitionTopN.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalPartitionTopN.java index 7666825f4e..2d214e4af4 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalPartitionTopN.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalPartitionTopN.java @@ -150,8 +150,9 @@ public class PhysicalPartitionTopN<CHILD_TYPE extends Plan> extends PhysicalUnar @Override public PhysicalPartitionTopN<Plan> withChildren(List<Plan> children) { Preconditions.checkArgument(children.size() == 1); - return new PhysicalPartitionTopN<>(function, partitionKeys, orderKeys, hasGlobalLimit, partitionLimit, - getLogicalProperties(), children.get(0)); + return new PhysicalPartitionTopN<>(function, partitionKeys, orderKeys, hasGlobalLimit, + partitionLimit, groupExpression, getLogicalProperties(), physicalProperties, + statistics, children.get(0)); } @Override diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalProject.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalProject.java index 3362d88ee6..f6e4d4dfdf 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalProject.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalProject.java @@ -103,7 +103,13 @@ public class PhysicalProject<CHILD_TYPE extends Plan> extends PhysicalUnary<CHIL @Override public PhysicalProject<Plan> withChildren(List<Plan> children) { Preconditions.checkArgument(children.size() == 1); - return new PhysicalProject<>(projects, getLogicalProperties(), children.get(0)); + return new PhysicalProject<Plan>(projects, + groupExpression, + getLogicalProperties(), + physicalProperties, + statistics, + children.get(0) + ); } @Override diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalRepeat.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalRepeat.java index 67b525214c..fdbe4beb6c 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalRepeat.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalRepeat.java @@ -147,7 +147,8 @@ public class PhysicalRepeat<CHILD_TYPE extends Plan> extends PhysicalUnary<CHILD @Override public PhysicalRepeat<Plan> withChildren(List<Plan> children) { Preconditions.checkArgument(children.size() == 1); - return new PhysicalRepeat<>(groupingSets, outputExpressions, getLogicalProperties(), children.get(0)); + return new PhysicalRepeat<>(groupingSets, outputExpressions, groupExpression, + getLogicalProperties(), physicalProperties, statistics, children.get(0)); } @Override diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalTopN.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalTopN.java index 4a58f5d9e9..06922cdbd9 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalTopN.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalTopN.java @@ -110,7 +110,8 @@ public class PhysicalTopN<CHILD_TYPE extends Plan> extends AbstractPhysicalSort< @Override public PhysicalTopN<Plan> withChildren(List<Plan> children) { Preconditions.checkArgument(children.size() == 1); - return new PhysicalTopN<>(orderKeys, limit, offset, phase, getLogicalProperties(), children.get(0)); + return new PhysicalTopN<>(orderKeys, limit, offset, phase, groupExpression, + getLogicalProperties(), physicalProperties, statistics, children.get(0)); } @Override diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalWindow.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalWindow.java index 8ac180fce8..82f548cdf0 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalWindow.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalWindow.java @@ -127,8 +127,8 @@ public class PhysicalWindow<CHILD_TYPE extends Plan> extends PhysicalUnary<CHILD @Override public Plan withChildren(List<Plan> children) { Preconditions.checkState(children.size() == 1); - return new PhysicalWindow<>(windowFrameGroup, requireProperties, Optional.empty(), - getLogicalProperties(), children.get(0)); + return new PhysicalWindow<>(windowFrameGroup, requireProperties, groupExpression, + getLogicalProperties(), physicalProperties, statistics, children.get(0)); } @Override diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/postprocess/PushdownFilterThroughProjectTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/postprocess/PushdownFilterThroughProjectTest.java new file mode 100644 index 0000000000..8c0e701e99 --- /dev/null +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/postprocess/PushdownFilterThroughProjectTest.java @@ -0,0 +1,112 @@ +// 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.postprocess; + +import org.apache.doris.catalog.KeysType; +import org.apache.doris.catalog.OlapTable; +import org.apache.doris.nereids.CascadesContext; +import org.apache.doris.nereids.processor.post.PushdownFilterThroughProject; +import org.apache.doris.nereids.properties.LogicalProperties; +import org.apache.doris.nereids.trees.expressions.Alias; +import org.apache.doris.nereids.trees.expressions.EqualTo; +import org.apache.doris.nereids.trees.expressions.Expression; +import org.apache.doris.nereids.trees.expressions.NamedExpression; +import org.apache.doris.nereids.trees.expressions.Slot; +import org.apache.doris.nereids.trees.expressions.SlotReference; +import org.apache.doris.nereids.trees.expressions.literal.Literal; +import org.apache.doris.nereids.trees.plans.ObjectId; +import org.apache.doris.nereids.trees.plans.PreAggStatus; +import org.apache.doris.nereids.trees.plans.physical.PhysicalFilter; +import org.apache.doris.nereids.trees.plans.physical.PhysicalOlapScan; +import org.apache.doris.nereids.trees.plans.physical.PhysicalPlan; +import org.apache.doris.nereids.trees.plans.physical.PhysicalProject; +import org.apache.doris.nereids.types.IntegerType; +import org.apache.doris.nereids.util.PlanConstructor; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Lists; +import com.google.common.collect.Sets; +import mockit.Injectable; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Optional; +import java.util.Set; + +public class PushdownFilterThroughProjectTest { + /** + * filter(y=0) + * | + * proj2(x as y, col2 as z, col3) + * | + * proj3(col1 as x, col2, col3) + * | + * SCAN(col1, col2, col3) + * + * transform to + * + * proj2(x as y, col2 as z, col3) + * | + * proj3(col1 as x, col2, col3) + * | + * filter(col1=0) + * | + * SCAN(col1, col2, col3) + * + */ + @Test + public void testPushFilter(@Injectable LogicalProperties placeHolder, + @Injectable CascadesContext ctx) { + OlapTable t1 = PlanConstructor.newOlapTable(0, "t1", 0, KeysType.DUP_KEYS); + List<String> qualifier = new ArrayList<>(); + qualifier.add("test"); + List<Slot> t1Output = new ArrayList<>(); + SlotReference a = new SlotReference("a", IntegerType.INSTANCE); + SlotReference b = new SlotReference("b", IntegerType.INSTANCE); + SlotReference c = new SlotReference("c", IntegerType.INSTANCE); + t1Output.add(a); + t1Output.add(b); + t1Output.add(c); + LogicalProperties t1Properties = new LogicalProperties(() -> t1Output); + PhysicalOlapScan scan = new PhysicalOlapScan(ObjectId.createGenerator().getNextId(), t1, + qualifier, 0L, Collections.emptyList(), Collections.emptyList(), null, + PreAggStatus.on(), ImmutableList.of(), Optional.empty(), t1Properties); + Alias x = new Alias(a, "x"); + List<NamedExpression> projList3 = Lists.newArrayList(x, b, c); + PhysicalProject proj3 = new PhysicalProject(projList3, placeHolder, scan); + Alias y = new Alias(x.toSlot(), "y"); + Alias z = new Alias(b, "z"); + List<NamedExpression> projList2 = Lists.newArrayList(y, z, c); + PhysicalProject proj2 = new PhysicalProject(projList2, placeHolder, proj3); + Set<Expression> conjuncts = Sets.newHashSet(); + conjuncts.add(new EqualTo(y.toSlot(), Literal.of(0))); + PhysicalFilter filter = new PhysicalFilter(conjuncts, proj2.getLogicalProperties(), proj2); + + PushdownFilterThroughProject processor = new PushdownFilterThroughProject(); + PhysicalPlan newPlan = (PhysicalPlan) filter.accept(processor, ctx); + Assertions.assertTrue(newPlan instanceof PhysicalProject); + Assertions.assertTrue(newPlan.child(0) instanceof PhysicalProject); + Assertions.assertTrue(newPlan.child(0).child(0) instanceof PhysicalFilter); + List<Expression> newFilterConjuncts = + ((PhysicalFilter<?>) newPlan.child(0).child(0)).getExpressions(); + Assertions.assertEquals(newFilterConjuncts.get(0).child(0), a); + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org