From abbd2cedff0760078a7c3253e2aa4066fadfca4c Mon Sep 17 00:00:00 2001 From: morrySnow <101034200+morrySnow@users.noreply.github.com> Date: Wed, 27 Dec 2023 21:04:00 +0800 Subject: [PATCH] [fix](Nereids) merge limit should use bottom phase (#29142) --- .../nereids/rules/rewrite/MergeLimits.java | 2 +- .../rules/rewrite/MergeLimitsTest.java | 50 +++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/MergeLimits.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/MergeLimits.java index 0621e70b70..e1428983c1 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/MergeLimits.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/MergeLimits.java @@ -50,7 +50,7 @@ public class MergeLimits extends OneRewriteRuleFactory { Math.min(upperLimit.getLimit(), Math.max(bottomLimit.getLimit() - upperLimit.getOffset(), 0)), bottomLimit.getOffset() + upperLimit.getOffset(), - upperLimit.getPhase(), bottomLimit.child() + upperLimit.child().getPhase(), bottomLimit.child() ); }).toRule(RuleType.MERGE_LIMITS); } diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/MergeLimitsTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/MergeLimitsTest.java index 9fcfd03e41..dbaeef6d79 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/MergeLimitsTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/MergeLimitsTest.java @@ -17,6 +17,8 @@ package org.apache.doris.nereids.rules.rewrite; +import org.apache.doris.nereids.trees.plans.LimitPhase; +import org.apache.doris.nereids.trees.plans.logical.LogicalLimit; import org.apache.doris.nereids.trees.plans.logical.LogicalOlapScan; import org.apache.doris.nereids.trees.plans.logical.LogicalPlan; import org.apache.doris.nereids.util.LogicalPlanBuilder; @@ -44,4 +46,52 @@ class MergeLimitsTest implements MemoPatternMatchSupported { .matches( logicalLimit().when(limit -> limit.getLimit() == 2).when(limit -> limit.getOffset() == 5)); } + + @Test + void testMergeLimitsWithLocalUpperAndGlobalBottom() { + LogicalPlan logicalLimit = new LogicalLimit<>(2, 0, LimitPhase.LOCAL, + new LogicalLimit<>(10, 0, LimitPhase.GLOBAL, scan1)); + + PlanChecker.from(new ConnectContext(), logicalLimit).applyTopDown(new MergeLimits()) + .matches( + logicalLimit().when(limit -> limit.getLimit() == 2) + .when(limit -> limit.getPhase() == LimitPhase.GLOBAL)); + } + + @Test + void testMergeLimitsWithLocalUpperAndLocalBottom() { + LogicalPlan logicalLimit = new LogicalLimit<>(2, 0, LimitPhase.LOCAL, + new LogicalLimit<>(10, 0, LimitPhase.LOCAL, scan1)); + + PlanChecker.from(new ConnectContext(), logicalLimit).applyTopDown(new MergeLimits()) + .matches( + logicalLimit().when(limit -> limit.getLimit() == 2) + .when(limit -> limit.getPhase() == LimitPhase.LOCAL)); + } + + @Test + void testMergeLimitsWithGlobalUpperAndGlobalBottom() { + LogicalPlan logicalLimit = new LogicalLimit<>(2, 0, LimitPhase.GLOBAL, + new LogicalLimit<>(10, 0, LimitPhase.GLOBAL, scan1)); + + PlanChecker.from(new ConnectContext(), logicalLimit).applyTopDown(new MergeLimits()) + .matches( + logicalLimit().when(limit -> limit.getLimit() == 2) + .when(limit -> limit.getPhase() == LimitPhase.GLOBAL)); + } + + @Test + void testMergeLimitsWithGlobalUpperAndLocalBottom() { + LogicalPlan logicalLimit = new LogicalLimit<>(2, 0, LimitPhase.GLOBAL, + new LogicalLimit<>(10, 0, LimitPhase.LOCAL, scan1)); + + PlanChecker.from(new ConnectContext(), logicalLimit).applyTopDown(new MergeLimits()) + .matches( + logicalLimit( + logicalLimit() + .when(limit -> limit.getLimit() == 10) + .when(limit -> limit.getPhase() == LimitPhase.LOCAL)) + .when(limit -> limit.getLimit() == 2) + .when(limit -> limit.getPhase() == LimitPhase.GLOBAL)); + } }