You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by kx...@apache.org on 2023/06/26 12:35:56 UTC

[doris] 03/09: [fix](nereids)change PushdownFilterThroughProject post processor from bottom up to top down rewrite (#21125)

This is an automated email from the ASF dual-hosted git repository.

kxiao pushed a commit to branch branch-2.0
in repository https://gitbox.apache.org/repos/asf/doris.git

commit 266750f7accda38d5a0bd15468126d8b8a2992a7
Author: starocean999 <40...@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 6b9c39fff1..f2c092515e 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
@@ -111,8 +111,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-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org