From c54d88ec55b7bf8955d1146b0173a258699bac54 Mon Sep 17 00:00:00 2001 From: minghong Date: Mon, 1 Dec 2025 15:17:50 +0800 Subject: [PATCH] [fix](nereids)fix pushdownLimit/topnThroughJoin dead loop (#58305) - PushDownTopNThroughJoin - PushDownLimitDistinctThroughJoin - PushDownTopNDistinctThroughJoin in pr#46773, we made above rules both applicable to RBO and CBO. In order to avoid dead loop, we have checked rewritten plan. But the check condition is not suitable to CBO. This leads optimizor apply these rules until group expression count reach memo_max_group_expression_size --- .../PushDownLimitDistinctThroughJoin.java | 14 ++++++++------ .../PushDownTopNDistinctThroughJoin.java | 11 +++++++++-- .../rewrite/PushDownTopNThroughJoin.java | 17 +++++++++++++++-- .../pre_rewrite/limit/query_with_limit.groovy | 19 +++++++++++++++++++ 4 files changed, 51 insertions(+), 10 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushDownLimitDistinctThroughJoin.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushDownLimitDistinctThroughJoin.java index 590e20a776366a..db538329339a6c 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushDownLimitDistinctThroughJoin.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushDownLimitDistinctThroughJoin.java @@ -21,6 +21,7 @@ import org.apache.doris.nereids.rules.RuleType; import org.apache.doris.nereids.trees.expressions.Slot; import org.apache.doris.nereids.trees.plans.Plan; +import org.apache.doris.nereids.trees.plans.algebra.Limit; import org.apache.doris.nereids.trees.plans.logical.LogicalAggregate; import org.apache.doris.nereids.trees.plans.logical.LogicalJoin; import org.apache.doris.nereids.trees.plans.logical.LogicalLimit; @@ -52,7 +53,7 @@ public List buildRules() { LogicalJoin join = agg.child(); Plan newJoin = pushLimitThroughJoin(limit, join); - if (newJoin == null || join.children().equals(newJoin.children())) { + if (newJoin == null) { return null; } return limit.withChildren(agg.withChildren(newJoin)); @@ -68,7 +69,7 @@ public List buildRules() { LogicalJoin join = project.child(); Plan newJoin = pushLimitThroughJoin(limit, join); - if (newJoin == null || join.children().equals(newJoin.children())) { + if (newJoin == null) { return null; } return limit.withChildren(agg.withChildren(project.withChildren(newJoin))); @@ -82,25 +83,26 @@ private Plan pushLimitThroughJoin(LogicalLimit limit, LogicalJoin .flatMap(e -> e.getInputSlots().stream()).collect(Collectors.toList()); switch (join.getJoinType()) { case LEFT_OUTER_JOIN: - if (join.left().getOutputSet().containsAll(groupBySlots) + if (!(join.left() instanceof Limit) + && join.left().getOutputSet().containsAll(groupBySlots) && join.left().getOutputSet().equals(agg.getOutputSet())) { return join.withChildren(limit.withLimitChild(limit.getLimit() + limit.getOffset(), 0, agg.withChildren(join.left())), join.right()); } return null; case RIGHT_OUTER_JOIN: - if (join.right().getOutputSet().containsAll(groupBySlots) + if (!(join.right() instanceof Limit) && join.right().getOutputSet().containsAll(groupBySlots) && join.right().getOutputSet().equals(agg.getOutputSet())) { return join.withChildren(join.left(), limit.withLimitChild(limit.getLimit() + limit.getOffset(), 0, agg.withChildren(join.right()))); } return null; case CROSS_JOIN: - if (join.left().getOutputSet().containsAll(groupBySlots) + if (!(join.left() instanceof Limit) && join.left().getOutputSet().containsAll(groupBySlots) && join.left().getOutputSet().equals(agg.getOutputSet())) { return join.withChildren(limit.withLimitChild(limit.getLimit() + limit.getOffset(), 0, agg.withChildren(join.left())), join.right()); - } else if (join.right().getOutputSet().containsAll(groupBySlots) + } else if (!(join.right() instanceof Limit) && join.right().getOutputSet().containsAll(groupBySlots) && join.right().getOutputSet().equals(agg.getOutputSet())) { return join.withChildren(join.left(), limit.withLimitChild(limit.getLimit() + limit.getOffset(), 0, agg.withChildren(join.right()))); diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushDownTopNDistinctThroughJoin.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushDownTopNDistinctThroughJoin.java index 6c280146c8ab2c..4c6f426f7ac1ba 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushDownTopNDistinctThroughJoin.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushDownTopNDistinctThroughJoin.java @@ -22,6 +22,7 @@ import org.apache.doris.nereids.rules.RuleType; import org.apache.doris.nereids.trees.expressions.Slot; import org.apache.doris.nereids.trees.plans.Plan; +import org.apache.doris.nereids.trees.plans.algebra.TopN; import org.apache.doris.nereids.trees.plans.logical.LogicalAggregate; import org.apache.doris.nereids.trees.plans.logical.LogicalJoin; import org.apache.doris.nereids.trees.plans.logical.LogicalProject; @@ -98,6 +99,9 @@ private Plan pushTopNThroughJoin(LogicalTopN topN, LogicalJoin

e.getInputSlots().stream()).collect(Collectors.toSet()); switch (join.getJoinType()) { case LEFT_OUTER_JOIN: { + if (join.left() instanceof TopN) { + return null; + } List pushedOrderKeys = getPushedOrderKeys(groupBySlots, join.left().getOutputSet(), topN.getOrderKeys()); if (!pushedOrderKeys.isEmpty()) { @@ -109,6 +113,9 @@ private Plan pushTopNThroughJoin(LogicalTopN topN, LogicalJoin

pushedOrderKeys = getPushedOrderKeys(groupBySlots, join.right().getOutputSet(), topN.getOrderKeys()); if (!pushedOrderKeys.isEmpty()) { @@ -124,14 +131,14 @@ private Plan pushTopNThroughJoin(LogicalTopN topN, LogicalJoin

leftPushedOrderKeys = getPushedOrderKeys(groupBySlots, join.left().getOutputSet(), topN.getOrderKeys()); - if (!leftPushedOrderKeys.isEmpty()) { + if (!(join.left() instanceof TopN) && !leftPushedOrderKeys.isEmpty()) { leftChild = topN.withLimitOrderKeyAndChild( topN.getLimit() + topN.getOffset(), 0, leftPushedOrderKeys, PlanUtils.distinct(join.left())); } List rightPushedOrderKeys = getPushedOrderKeys(groupBySlots, join.right().getOutputSet(), topN.getOrderKeys()); - if (!rightPushedOrderKeys.isEmpty()) { + if (!(join.right() instanceof TopN) && !rightPushedOrderKeys.isEmpty()) { rightChild = topN.withLimitOrderKeyAndChild( topN.getLimit() + topN.getOffset(), 0, rightPushedOrderKeys, PlanUtils.distinct(join.right())); diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushDownTopNThroughJoin.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushDownTopNThroughJoin.java index 4d15dae59e76da..7205784d0250e4 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushDownTopNThroughJoin.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushDownTopNThroughJoin.java @@ -22,6 +22,7 @@ import org.apache.doris.nereids.rules.RuleType; import org.apache.doris.nereids.trees.expressions.Slot; import org.apache.doris.nereids.trees.plans.Plan; +import org.apache.doris.nereids.trees.plans.algebra.TopN; import org.apache.doris.nereids.trees.plans.logical.LogicalJoin; import org.apache.doris.nereids.trees.plans.logical.LogicalProject; import org.apache.doris.nereids.trees.plans.logical.LogicalTopN; @@ -53,7 +54,7 @@ public List buildRules() { .then(topN -> { LogicalJoin join = topN.child(); Plan newJoin = pushLimitThroughJoin(topN, join); - if (newJoin == null || topN.child().children().equals(newJoin.children())) { + if (newJoin == null) { return null; } return topN.withChildren(newJoin); @@ -80,7 +81,7 @@ public List buildRules() { } Plan newJoin = pushLimitThroughJoin(topN, join); - if (newJoin == null || join.children().equals(newJoin.children())) { + if (newJoin == null) { return null; } return topN.withChildren(project.withChildren(newJoin)); @@ -93,6 +94,9 @@ private Plan pushLimitThroughJoin(LogicalTopN topN, LogicalJoin< .flatMap(e -> e.getInputSlots().stream()).collect(Collectors.toList()); switch (join.getJoinType()) { case LEFT_OUTER_JOIN: + if (join.left() instanceof TopN) { + return null; + } if (join.left().getOutputSet().containsAll(orderbySlots)) { return join.withChildren( topN.withLimitChild(topN.getLimit() + topN.getOffset(), 0, join.left()), @@ -100,6 +104,9 @@ private Plan pushLimitThroughJoin(LogicalTopN topN, LogicalJoin< } return null; case RIGHT_OUTER_JOIN: + if (join.right() instanceof TopN) { + return null; + } if (join.right().getOutputSet().containsAll(orderbySlots)) { return join.withChildren( join.left(), @@ -108,10 +115,16 @@ private Plan pushLimitThroughJoin(LogicalTopN topN, LogicalJoin< return null; case CROSS_JOIN: if (join.left().getOutputSet().containsAll(orderbySlots)) { + if (join.left() instanceof TopN) { + return null; + } return join.withChildren( topN.withLimitChild(topN.getLimit() + topN.getOffset(), 0, join.left()), join.right()); } else if (join.right().getOutputSet().containsAll(orderbySlots)) { + if (join.right() instanceof TopN) { + return null; + } return join.withChildren( join.left(), topN.withLimitChild(topN.getLimit() + topN.getOffset(), 0, join.right())); diff --git a/regression-test/suites/nereids_rules_p0/mv/pre_rewrite/limit/query_with_limit.groovy b/regression-test/suites/nereids_rules_p0/mv/pre_rewrite/limit/query_with_limit.groovy index 5e29a5ef2afbce..22a4b7e198722d 100644 --- a/regression-test/suites/nereids_rules_p0/mv/pre_rewrite/limit/query_with_limit.groovy +++ b/regression-test/suites/nereids_rules_p0/mv/pre_rewrite/limit/query_with_limit.groovy @@ -137,6 +137,25 @@ suite("query_with_limit") { sql """alter table orders modify column O_COMMENT set stats ('row_count'='8');""" sql """alter table partsupp modify column ps_comment set stats ('row_count'='2');""" + explain { + sql """ + select + o_orderdate, + o_shippriority, + o_comment, + l_orderkey, + l_partkey + from + orders left + join lineitem on l_orderkey = o_orderkey + left join partsupp on ps_partkey = l_partkey and l_suppkey = ps_suppkey + order by l_orderkey + limit 2 + offset 1; + """ + notContains "group expression count exceeds memo_max_group_expression_size(10000)" + } + // test limit without offset def mv1_0 = """