Skip to content

Commit 2242ae9

Browse files
branch-4.1: [fix](nereids) clamp the merged limit of MERGE_TOP_N by the parent offset #64306 (#64353)
Cherry-picked from #64306 Co-authored-by: 924060929 <lanhuajian@selectdb.com>
1 parent a49f83c commit 2242ae9

4 files changed

Lines changed: 87 additions & 4 deletions

File tree

fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/MergeTopNs.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@
3030
* <p>
3131
* topN - child topN
3232
* If child topN orderby list is prefix subList of topN =>
33-
* topN with limit = min(topN.limit, childTopN.limit), offset = topN.offset + childTopN.offset
33+
* topN with limit = min(topN.limit, max(childTopN.limit - topN.offset, 0)),
34+
* offset = topN.offset + childTopN.offset
3435
*/
3536
public class MergeTopNs extends OneRewriteRuleFactory {
3637
@Override
@@ -48,8 +49,12 @@ public Rule build() {
4849
long childOffset = childTopN.getOffset();
4950
long childLimit = childTopN.getLimit();
5051
long newOffset = offset + childOffset;
51-
// choose min limit
52-
long newLimit = Math.min(limit, childLimit);
52+
// The parent's offset is applied on top of the child's output, so only
53+
// (childLimit - offset) of the child's rows survive. Clamp the merged limit
54+
// by that remaining count; otherwise rows beyond the child's limit leak through
55+
// when the parent has a non-zero offset (DORIS-26301). Same semantics as
56+
// MergeLimits.mergeLimit for consecutive limits.
57+
long newLimit = Math.min(limit, Math.max(childLimit - offset, 0));
5358
return topN.withLimitChild(newLimit, newOffset, childTopN.child());
5459
}).toRule(RuleType.MERGE_TOP_N);
5560
}

fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/MergeTopNsTest.java

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,25 @@ void testOffset() {
7070
PlanChecker.from(MemoTestUtils.createConnectContext(), plan)
7171
.applyTopDown(new MergeTopNs())
7272
.matches(
73-
logicalTopN(logicalOlapScan()).when(topN -> topN.getLimit() == 10 && topN.getOffset() == 4)
73+
logicalTopN(logicalOlapScan()).when(topN -> topN.getLimit() == 9 && topN.getOffset() == 4)
74+
);
75+
}
76+
77+
@Test
78+
void testParentOffsetReducesChildLimit() {
79+
// DORIS-26301: when the parent (upper) TopN has a non-zero offset, the merged limit must be
80+
// clamped by (childLimit - parentOffset); otherwise rows beyond the child's limit leak through.
81+
// child : ORDER BY k LIMIT 5 (limit=5, offset=0) -> yields 5 rows
82+
// parent: ORDER BY k LIMIT 3 OFFSET 4 (limit=3, offset=4) -> skips 4, only 1 row remains
83+
// merged: offset = 4, limit = min(3, max(5 - 4, 0)) = 1
84+
LogicalPlan plan = new LogicalPlanBuilder(score)
85+
.topN(5, 0, ImmutableList.of(0))
86+
.topN(3, 4, ImmutableList.of(0))
87+
.build();
88+
PlanChecker.from(MemoTestUtils.createConnectContext(), plan)
89+
.applyTopDown(new MergeTopNs())
90+
.matches(
91+
logicalTopN(logicalOlapScan()).when(topN -> topN.getLimit() == 1 && topN.getOffset() == 4)
7492
);
7593
}
7694

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
-- This file is automatically generated. You should know what you did if you want to edit this
2+
-- !merge_topn_off4lim3 --
3+
5 50
4+
5+
-- !merge_topn_off2lim3 --
6+
3 30
7+
4 40
8+
5 50
9+
10+
-- !merge_topn_off6lim3 --
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
suite("test_merge_topn_offset") {
19+
// DORIS-26301: when MERGE_TOP_N merges a parent TopN that carries a non-zero OFFSET into its
20+
// child TopN, the merged limit must be clamped by (childLimit - parentOffset). Otherwise rows
21+
// beyond the child's limit leak through. Before the fix, an outer "LIMIT 3 OFFSET 4" over an
22+
// inner "LIMIT 5" wrongly returned 3 rows (k=5,6,7) instead of the single correct row (k=5).
23+
sql "SET enable_nereids_planner=true"
24+
sql "SET enable_fallback_to_original_planner=false"
25+
26+
def tableName = "test_merge_topn_offset"
27+
sql """ DROP TABLE IF EXISTS ${tableName} """
28+
sql """
29+
CREATE TABLE ${tableName} (
30+
k INT NOT NULL,
31+
v INT NOT NULL
32+
)
33+
DUPLICATE KEY(k)
34+
DISTRIBUTED BY HASH(k) BUCKETS 1
35+
PROPERTIES ("replication_num" = "1")
36+
"""
37+
sql """ INSERT INTO ${tableName} VALUES
38+
(1, 10), (2, 20), (3, 30), (4, 40), (5, 50),
39+
(6, 60), (7, 70), (8, 80), (9, 90), (10, 100) """
40+
41+
// MERGE_TOP_N is enabled by default; stacking an outer TopN with an OFFSET over an inner TopN
42+
// triggers it. The inner "ORDER BY k LIMIT 5" yields k = 1..5.
43+
44+
// outer offset (4) consumes 4 of the inner's 5 rows -> only k=5 remains
45+
qt_merge_topn_off4lim3 """ SELECT * FROM (SELECT k, v FROM ${tableName} ORDER BY k LIMIT 5) s ORDER BY k LIMIT 3 OFFSET 4 """
46+
// outer offset (2) is within the inner limit -> k = 3,4,5
47+
qt_merge_topn_off2lim3 """ SELECT * FROM (SELECT k, v FROM ${tableName} ORDER BY k LIMIT 5) s ORDER BY k LIMIT 3 OFFSET 2 """
48+
// outer offset (6) exceeds the inner limit (5) -> empty
49+
qt_merge_topn_off6lim3 """ SELECT * FROM (SELECT k, v FROM ${tableName} ORDER BY k LIMIT 5) s ORDER BY k LIMIT 3 OFFSET 6 """
50+
}

0 commit comments

Comments
 (0)