[fix](nereids) topN filter: use ObjectId as map key instead of Topn n…

…ode (apache#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: apache#31485
morningman · Jan 8, 2025 · 811f936 · 811f936
1 parent c3ed2d3
commit 811f936
Show file tree

Hide file tree

Showing 6 changed files with 35 additions and 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
@@ -21,6 +21,7 @@
 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 @@
  * 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 void translateTarget(PhysicalRelation relation, ScanNode legacyScan,
      * 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 String toString() {
         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
@@ -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/...c/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalDeferMaterializeTopN.java b/...c/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalDeferMaterializeTopN.java
@@ -25,6 +25,7 @@
 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 void computeFd(DataTrait.Builder builder) {
     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
@@ -23,6 +23,7 @@
 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 void computeEqualSet(DataTrait.Builder builder) {
     public void computeFd(DataTrait.Builder builder) {
         builder.addFuncDepsDG(child().getLogicalProperties().getTrait());
     }
+
+    @Override
+    public ObjectId getObjectId() {
+        return id;
+    }
 }
diff --git a/...main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalDeferMaterializeTopN.java b/...main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalDeferMaterializeTopN.java
@@ -23,6 +23,7 @@
 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 String toString() {
                 "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
@@ -22,6 +22,7 @@
 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 PhysicalTopN<Plan> resetLogicalProperties() {
                 null, physicalProperties, statistics, child());
     }
 
+    @Override
+    public ObjectId getObjectId() {
+        return id;
+    }
 }