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