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

englefly 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 811f93682d5 [fix](nereids) topN filter: use ObjectId as map key 
instead of Topn node  (#46551)
811f93682d5 is described below

commit 811f93682d58eae76f87c411fcb2a572f03b2c92
Author: minghong <zhoumingh...@selectdb.com>
AuthorDate: Wed Jan 8 10:25:44 2025 +0800

    [fix](nereids) topN filter: use ObjectId as map key instead of Topn node  
(#46551)
    
    ### What problem does this PR solve?
    Plan node is not good to be hash map key, because two plan nodes in
    different tree level may be regarded as "equal". for example, in
    following tree, topn1.equals(topn2) may be true.
     Topn filter generator should distinguish them, and hence topn
    node is not suitable to be used as hash map key.
    
    topn1
        -->some node
            -->topn2
                 -->other node
    
    Related PR: #31485
---
 .../doris/nereids/processor/post/TopnFilterContext.java   | 15 ++++++++-------
 .../apache/doris/nereids/trees/plans/algebra/TopN.java    |  4 ++++
 .../trees/plans/logical/LogicalDeferMaterializeTopN.java  |  6 ++++++
 .../doris/nereids/trees/plans/logical/LogicalTopN.java    |  6 ++++++
 .../plans/physical/PhysicalDeferMaterializeTopN.java      |  6 ++++++
 .../doris/nereids/trees/plans/physical/PhysicalTopN.java  |  5 +++++
 6 files changed, 35 insertions(+), 7 deletions(-)

diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/TopnFilterContext.java
 
b/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/TopnFilterContext.java
index fceec21ee7e..5df829e5796 100644
--- 
a/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/TopnFilterContext.java
+++ 
b/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/TopnFilterContext.java
@@ -21,6 +21,7 @@ import org.apache.doris.analysis.Expr;
 import org.apache.doris.nereids.glue.translator.ExpressionTranslator;
 import org.apache.doris.nereids.glue.translator.PlanTranslatorContext;
 import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.plans.ObjectId;
 import org.apache.doris.nereids.trees.plans.algebra.TopN;
 import org.apache.doris.nereids.trees.plans.physical.PhysicalRelation;
 import org.apache.doris.nereids.trees.plans.physical.TopnFilter;
@@ -38,22 +39,22 @@ import java.util.Map;
  * topN runtime filter context
  */
 public class TopnFilterContext {
-    private final Map<TopN, TopnFilter> filters = Maps.newHashMap();
+    private final Map<ObjectId, TopnFilter> filters = Maps.newHashMap();
 
     /**
      * add topN filter
      */
     public void addTopnFilter(TopN topn, PhysicalRelation scan, Expression 
expr) {
-        TopnFilter filter = filters.get(topn);
+        TopnFilter filter = filters.get(topn.getObjectId());
         if (filter == null) {
-            filters.put(topn, new TopnFilter(topn, scan, expr));
+            filters.put(topn.getObjectId(), new TopnFilter(topn, scan, expr));
         } else {
             filter.addTarget(scan, expr);
         }
     }
 
     public boolean isTopnFilterSource(TopN topn) {
-        return filters.containsKey(topn);
+        return filters.containsKey(topn.getObjectId());
     }
 
     public List<TopnFilter> getTopnFilters() {
@@ -77,7 +78,7 @@ public class TopnFilterContext {
      * translate topn-filter
      */
     public void translateSource(TopN topn, SortNode sortNode) {
-        TopnFilter filter = filters.get(topn);
+        TopnFilter filter = filters.get(topn.getObjectId());
         if (filter == null) {
             return;
         }
@@ -97,8 +98,8 @@ public class TopnFilterContext {
         String indent = "   ";
         String arrow = " -> ";
         builder.append("filters:\n");
-        for (TopN topn : filters.keySet()) {
-            
builder.append(indent).append(arrow).append(filters.get(topn)).append("\n");
+        for (ObjectId topnId : filters.keySet()) {
+            
builder.append(indent).append(arrow).append(filters.get(topnId)).append("\n");
         }
         return builder.toString();
     }
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/algebra/TopN.java
 
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/algebra/TopN.java
index c214dffbbf0..ed4b71ee91a 100644
--- 
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/algebra/TopN.java
+++ 
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/algebra/TopN.java
@@ -17,6 +17,8 @@
 
 package org.apache.doris.nereids.trees.plans.algebra;
 
+import org.apache.doris.nereids.trees.plans.ObjectId;
+
 /**
  * Common interface for logical/physical TopN.
  */
@@ -25,4 +27,6 @@ public interface TopN extends Sort {
     long getOffset();
 
     long getLimit();
+
+    ObjectId getObjectId();
 }
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalDeferMaterializeTopN.java
 
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalDeferMaterializeTopN.java
index 9a0d03b52a1..5a791def8ee 100644
--- 
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalDeferMaterializeTopN.java
+++ 
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalDeferMaterializeTopN.java
@@ -25,6 +25,7 @@ import org.apache.doris.nereids.trees.expressions.ExprId;
 import org.apache.doris.nereids.trees.expressions.Expression;
 import org.apache.doris.nereids.trees.expressions.Slot;
 import org.apache.doris.nereids.trees.expressions.SlotReference;
+import org.apache.doris.nereids.trees.plans.ObjectId;
 import org.apache.doris.nereids.trees.plans.Plan;
 import org.apache.doris.nereids.trees.plans.PlanType;
 import org.apache.doris.nereids.trees.plans.algebra.TopN;
@@ -198,4 +199,9 @@ public class LogicalDeferMaterializeTopN<CHILD_TYPE extends 
Plan> extends Logica
     public void computeEqualSet(DataTrait.Builder builder) {
         builder.addEqualSet(child().getLogicalProperties().getTrait());
     }
+
+    @Override
+    public ObjectId getObjectId() {
+        return id;
+    }
 }
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalTopN.java
 
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalTopN.java
index 9c7cfacad8d..6d6aa2b8131 100644
--- 
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalTopN.java
+++ 
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalTopN.java
@@ -23,6 +23,7 @@ import org.apache.doris.nereids.properties.LogicalProperties;
 import org.apache.doris.nereids.properties.OrderKey;
 import org.apache.doris.nereids.trees.expressions.Expression;
 import org.apache.doris.nereids.trees.expressions.Slot;
+import org.apache.doris.nereids.trees.plans.ObjectId;
 import org.apache.doris.nereids.trees.plans.Plan;
 import org.apache.doris.nereids.trees.plans.PlanType;
 import org.apache.doris.nereids.trees.plans.algebra.TopN;
@@ -186,4 +187,9 @@ public class LogicalTopN<CHILD_TYPE extends Plan> extends 
LogicalUnary<CHILD_TYP
     public void computeFd(DataTrait.Builder builder) {
         builder.addFuncDepsDG(child().getLogicalProperties().getTrait());
     }
+
+    @Override
+    public ObjectId getObjectId() {
+        return id;
+    }
 }
diff --git 
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalDeferMaterializeTopN.java
 
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalDeferMaterializeTopN.java
index f5db3ff42f8..74088498d72 100644
--- 
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalDeferMaterializeTopN.java
+++ 
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalDeferMaterializeTopN.java
@@ -23,6 +23,7 @@ import org.apache.doris.nereids.properties.PhysicalProperties;
 import org.apache.doris.nereids.trees.expressions.ExprId;
 import org.apache.doris.nereids.trees.expressions.Slot;
 import org.apache.doris.nereids.trees.expressions.SlotReference;
+import org.apache.doris.nereids.trees.plans.ObjectId;
 import org.apache.doris.nereids.trees.plans.Plan;
 import org.apache.doris.nereids.trees.plans.algebra.TopN;
 import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor;
@@ -175,4 +176,9 @@ public class PhysicalDeferMaterializeTopN<CHILD_TYPE 
extends Plan>
                 "columnIdSlot", columnIdSlot
         );
     }
+
+    @Override
+    public ObjectId getObjectId() {
+        return id;
+    }
 }
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 c387a58dd0c..5f59f8a7083 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
@@ -22,6 +22,7 @@ import org.apache.doris.nereids.properties.LogicalProperties;
 import org.apache.doris.nereids.properties.OrderKey;
 import org.apache.doris.nereids.properties.PhysicalProperties;
 import org.apache.doris.nereids.trees.expressions.Slot;
+import org.apache.doris.nereids.trees.plans.ObjectId;
 import org.apache.doris.nereids.trees.plans.Plan;
 import org.apache.doris.nereids.trees.plans.PlanType;
 import org.apache.doris.nereids.trees.plans.SortPhase;
@@ -162,4 +163,8 @@ public class PhysicalTopN<CHILD_TYPE extends Plan> extends 
AbstractPhysicalSort<
                 null, physicalProperties, statistics, child());
     }
 
+    @Override
+    public ObjectId getObjectId() {
+        return id;
+    }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to