fix cases

Signed-off-by: stdpain <drfeng08@gmail.com>
StarRocks · Dec 31, 2024 · a593de2 · a593de2
1 parent 82aa03b
commit a593de2
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 6 deletions.
diff --git a/fe/fe-core/src/main/java/com/starrocks/planner/ExchangeNode.java b/fe/fe-core/src/main/java/com/starrocks/planner/ExchangeNode.java
@@ -143,6 +143,13 @@ public boolean isMerge() {
         return mergeInfo != null;
     }
 
+    public long getOffset() {
+        return offset;
+    }
+    public void setOffset(long offset) {
+        this.offset = offset;
+    }
+
     public void setReceiveColumns(List<Integer> receiveColumns) {
         this.receiveColumns = receiveColumns;
     }

diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/plan/PlanFragmentBuilder.java b/fe/fe-core/src/main/java/com/starrocks/sql/plan/PlanFragmentBuilder.java
@@ -3368,7 +3368,9 @@ public PlanFragment visitPhysicalLimit(OptExpression optExpression, ExecPlan con
             // now sr only support offset on merge-exchange node
             // 1. if child is exchange node, meanings child was enforced gather property
             //   a. only limit and no offset, only need set limit on child
-            //   b. has offset, should trans Exchange to Merge-Exchange node
+            //   b. has offset
+            //      b.1 not merge exchange should trans Exchange to Merge-Exchange node
+            //      b.2 is merge exchange update offset
             // 2. if child isn't exchange node, meanings child satisfy gather property
             //   a. only limit and no offset, no need add exchange node, only need set limit on child
             //   b. has offset, need add exchange node, sr doesn't support a special node to handle offset
@@ -3392,12 +3394,16 @@ public PlanFragment visitPhysicalLimit(OptExpression optExpression, ExecPlan con
                     child = fragment;
                 }
                 ExchangeNode exchangeNode = (ExchangeNode) child.getPlanRoot();
-                SortInfo sortInfo = new SortInfo(Lists.newArrayList(), Operator.DEFAULT_LIMIT,
-                        Lists.newArrayList(new IntLiteral(1)), Lists.newArrayList(true), Lists.newArrayList(false));
-                // we don't have to assign sort info when exchange node has already have sort info
                 if (!exchangeNode.isMerge()) {
+                    SortInfo sortInfo = new SortInfo(Lists.newArrayList(), Operator.DEFAULT_LIMIT,
+                            Lists.newArrayList(new IntLiteral(1)), Lists.newArrayList(true), Lists.newArrayList(false));
                     exchangeNode.setMergeInfo(sortInfo, limit.getOffset());
+                } else if (exchangeNode.getOffset() <= 0) {
+                    exchangeNode.setOffset(limit.getOffset());
+                } else if (exchangeNode.getOffset() > 0) {
+                    exchangeNode.setOffset(limit.getOffset() + exchangeNode.getOffset());
                 }
+
                 exchangeNode.computeStatistics(optExpression.getStatistics());
             }
 

diff --git a/fe/fe-core/src/test/java/com/starrocks/sql/plan/LimitTest.java b/fe/fe-core/src/test/java/com/starrocks/sql/plan/LimitTest.java
@@ -636,14 +636,23 @@ public void testLimitPushDownJoin() throws Exception {
 
     @Test
     public void testOffsetWithSubTopN() throws Exception {
-        String sql = "select v1 from (\n" +
+        String sql;
+        String plan;
+        sql = "select v1 from (\n" +
                 "  select * from (select v1, v2 from t0 order by v1 asc limit 1000, 600) l\n" +
                 "  left join (select null as cx, '1' as c1) r\n" +
                 "  on l.v1 =r.cx\n" +
                 ") b limit 600;";
-        String plan = getThriftPlan(sql);
+        plan = getThriftPlan(sql);
         assertContains(plan, "TExchangeNode(input_row_tuples:[1], sort_info:" +
                 "TSortInfo(ordering_exprs:[TExpr(nodes:[TExprNode(node_type:SLOT_REF");
+
+        sql = "select * from (select v1, v2 from t0 order by v1 asc limit 1000, 600) l limit 200, 600";
+        plan = getFragmentPlan(sql);
+        assertContains(plan, "\n" +
+                "  2:MERGING-EXCHANGE\n" +
+                "     offset: 1200\n" +
+                "     limit: 400");
     }
 
     @Test