This is an automated email from the ASF dual-hosted git repository.
morrysnow pushed a commit to branch branch-3.0
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-3.0 by this push:
new 4ce294fc763 [fix](Nereids) fixed the limit offset error (#39316)
(#41967)
4ce294fc763 is described below
commit 4ce294fc763eba29f3e1f32e2cff4b10109c3c54
Author: John Zhang <zhangzhiyon...@126.com>
AuthorDate: Thu Oct 17 18:20:02 2024 +0800
[fix](Nereids) fixed the limit offset error (#39316) (#41967)
cherry-pick from master #39316
---
.../glue/translator/PhysicalPlanTranslator.java | 36 ++++++++++++++++---
.../processor/post/AddOffsetIntoDistribute.java | 42 ----------------------
.../nereids/processor/post/PlanPostProcessors.java | 1 -
.../nereids_syntax_p0/sub_query_correlated.out | 9 -----
.../data/nereids_syntax_p0/test_limit.out | 6 ++++
.../suites/nereids_syntax_p0/test_limit.groovy | 42 +++++++++++++++++++---
6 files changed, 76 insertions(+), 60 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java
index c889a2a75a3..a49e210aa44 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java
@@ -1852,10 +1852,38 @@ public class PhysicalPlanTranslator extends
DefaultPlanVisitor<PlanFragment, Pla
public PlanFragment visitPhysicalLimit(PhysicalLimit<? extends Plan>
physicalLimit, PlanTranslatorContext context) {
PlanFragment inputFragment = physicalLimit.child(0).accept(this,
context);
PlanNode child = inputFragment.getPlanRoot();
- child.setLimit(MergeLimits.mergeLimit(physicalLimit.getLimit(),
physicalLimit.getOffset(), child.getLimit()));
- // TODO: plan node don't support limit
- // child.setOffset(MergeLimits.mergeOffset(physicalLimit.getOffset(),
child.getOffset()));
- updateLegacyPlanIdToPhysicalPlan(child, physicalLimit);
+
+ if (physicalLimit.getPhase().isLocal()) {
+ child.setLimit(MergeLimits.mergeLimit(physicalLimit.getLimit(),
physicalLimit.getOffset(),
+ child.getLimit()));
+ } else if (physicalLimit.getPhase().isGlobal()) {
+ if (!(child instanceof ExchangeNode)) {
+ ExchangeNode exchangeNode = new
ExchangeNode(context.nextPlanNodeId(), child);
+ exchangeNode.setLimit(physicalLimit.getLimit());
+ exchangeNode.setOffset(physicalLimit.getOffset());
+ exchangeNode.setPartitionType(TPartitionType.UNPARTITIONED);
+ exchangeNode.setNumInstances(1);
+
+ PlanFragment fragment = new
PlanFragment(context.nextFragmentId(), exchangeNode,
+ DataPartition.UNPARTITIONED);
+ inputFragment.setDestination(exchangeNode);
+ inputFragment.setOutputPartition(DataPartition.UNPARTITIONED);
+
+ DataStreamSink sink = new DataStreamSink(exchangeNode.getId());
+ sink.setOutputPartition(DataPartition.UNPARTITIONED);
+ inputFragment.setSink(sink);
+
+ context.addPlanFragment(fragment);
+ inputFragment = fragment;
+ } else {
+ ExchangeNode exchangeNode = (ExchangeNode) child;
+
exchangeNode.setLimit(MergeLimits.mergeLimit(physicalLimit.getLimit(),
physicalLimit.getOffset(),
+ exchangeNode.getLimit()));
+
exchangeNode.setOffset(MergeLimits.mergeOffset(physicalLimit.getOffset(),
exchangeNode.getOffset()));
+ }
+ }
+
+ updateLegacyPlanIdToPhysicalPlan(inputFragment.getPlanRoot(),
physicalLimit);
return inputFragment;
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/AddOffsetIntoDistribute.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/AddOffsetIntoDistribute.java
deleted file mode 100644
index dc817321298..00000000000
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/AddOffsetIntoDistribute.java
+++ /dev/null
@@ -1,42 +0,0 @@
-// Licensed to the Apache Software Foundation (ASF) under one
-// or more contributor license agreements. See the NOTICE file
-// distributed with this work for additional information
-// regarding copyright ownership. The ASF licenses this file
-// to you under the Apache License, Version 2.0 (the
-// "License"); you may not use this file except in compliance
-// with the License. You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing,
-// software distributed under the License is distributed on an
-// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-// KIND, either express or implied. See the License for the
-// specific language governing permissions and limitations
-// under the License.
-
-package org.apache.doris.nereids.processor.post;
-
-import org.apache.doris.nereids.CascadesContext;
-import org.apache.doris.nereids.properties.DistributionSpecGather;
-import org.apache.doris.nereids.trees.plans.Plan;
-import org.apache.doris.nereids.trees.plans.physical.PhysicalDistribute;
-import org.apache.doris.nereids.trees.plans.physical.PhysicalLimit;
-
-/**
- * Offset just can be in exchangeNode.
- * So, `offset` action is after `limit` action.
- * So, `limit` should update with `offset + limit`
- */
-public class AddOffsetIntoDistribute extends PlanPostProcessor {
- @Override
- public Plan visitPhysicalLimit(PhysicalLimit<? extends Plan> limit,
CascadesContext context) {
- limit = (PhysicalLimit<? extends Plan>) super.visit(limit, context);
- if (limit.getPhase().isLocal() || limit.getOffset() == 0) {
- return limit;
- }
-
- return new PhysicalDistribute<>(DistributionSpecGather.INSTANCE,
- limit.withLimit(limit.getLimit() +
limit.getOffset())).copyStatsAndGroupIdFrom(limit);
- }
-}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/PlanPostProcessors.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/PlanPostProcessors.java
index 11a4b73d8a3..a8654e27291 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/PlanPostProcessors.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/PlanPostProcessors.java
@@ -62,7 +62,6 @@ public class PlanPostProcessors {
builder.add(new RemoveUselessProjectPostProcessor());
builder.add(new MergeProjectPostProcessor());
builder.add(new RecomputeLogicalPropertiesProcessor());
- builder.add(new AddOffsetIntoDistribute());
if
(cascadesContext.getConnectContext().getSessionVariable().enableAggregateCse) {
builder.add(new ProjectAggregateExpressionsForCse());
}
diff --git a/regression-test/data/nereids_syntax_p0/sub_query_correlated.out
b/regression-test/data/nereids_syntax_p0/sub_query_correlated.out
index e7758d02a5f..d57a673339b 100644
--- a/regression-test/data/nereids_syntax_p0/sub_query_correlated.out
+++ b/regression-test/data/nereids_syntax_p0/sub_query_correlated.out
@@ -187,15 +187,6 @@
-- !exist_corr_limit0 --
-- !exist_unCorrelated_limit1_offset1 --
-1 2
-1 3
-2 4
-2 5
-3 3
-3 4
-20 2
-22 3
-24 4
-- !exist_unCorrelated_limit0_offset1 --
diff --git a/regression-test/data/nereids_syntax_p0/test_limit.out
b/regression-test/data/nereids_syntax_p0/test_limit.out
new file mode 100644
index 00000000000..f36542a7375
--- /dev/null
+++ b/regression-test/data/nereids_syntax_p0/test_limit.out
@@ -0,0 +1,6 @@
+-- This file is automatically generated. You should know what you did if you
want to edit this
+-- !limit1 --
+2 7844 TURNER SALESMAN 7698 1981-09-08 1500.0 0.0
30
+
+-- !lmit2 --
+3 7934 MILLER CLERK 7782 1982-01-23 1300.0 0.0 10
diff --git a/regression-test/suites/nereids_syntax_p0/test_limit.groovy
b/regression-test/suites/nereids_syntax_p0/test_limit.groovy
index 64e48195a17..fb24d8f1a3a 100644
--- a/regression-test/suites/nereids_syntax_p0/test_limit.groovy
+++ b/regression-test/suites/nereids_syntax_p0/test_limit.groovy
@@ -16,10 +16,6 @@
// under the License.
suite("test_limit") {
- sql 'set enable_nereids_planner=true'
- sql 'set enable_fallback_to_original_planner=false'
-
-
sql """
drop table if exists test1
"""
@@ -32,8 +28,46 @@ suite("test_limit") {
sql """ insert into test1 values(1) """
sql """ insert into test1 values(1) """
+
test {
sql "select * from test1 limit 2 offset 1"
result([[1]])
}
+
+ sql """
+ drop table if exists row_number_limit_tbl;
+ """
+ sql """
+ CREATE TABLE row_number_limit_tbl (
+ k1 INT NULL,
+ k2 VARCHAR(255) NULL,
+ k3 VARCHAR(255) NULL,
+ k4 INT NULL,
+ k5 VARCHAR(255) NULL,
+ k6 FLOAT NULL,
+ k7 FLOAT NULL,
+ k8 INT NULL
+ ) ENGINE=OLAP
+ DUPLICATE KEY(k1, k2)
+ DISTRIBUTED BY HASH(k1) BUCKETS 3
+ PROPERTIES (
+ "replication_allocation" = "tag.location.default: 1"
+ );
+ """
+
+ sql """ INSERT INTO row_number_limit_tbl VALUES (7788, 'SCOTT', 'ANALYST',
7566, '1987-04-19', 3000, 0, 20); """
+ sql """ INSERT INTO row_number_limit_tbl VALUES (7844, 'TURNER',
'SALESMAN', 7698, '1981-09-08', 1500, 0, 30); """
+ qt_limit1 """
+ select row_number() over(order by k6 desc) k6s, t.* from
row_number_limit_tbl t order by k6s limit 1 offset 1;
+ """
+
+ sql """ truncate table row_number_limit_tbl; """
+
+ sql """ INSERT INTO row_number_limit_tbl VALUES (7788, 'SCOTT', 'ANALYST',
7566, '1987-04-19', 3000, 0, 20); """
+ sql """ INSERT INTO row_number_limit_tbl VALUES (7844, 'TURNER',
'SALESMAN', 7698, '1981-09-08', 1500, 0, 30); """
+ sql """ INSERT INTO row_number_limit_tbl VALUES (7934, 'MILLER', 'CLERK',
7782, '1982-01-23', 1300, 0, 10); """
+
+ qt_lmit2 """
+ select row_number() over(order by k6 desc) k6s, t.* from
row_number_limit_tbl t limit 1 offset 2;
+ """
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
