This is an automated email from the ASF dual-hosted git repository.
morrySnow pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new ed0a95d7005 [fix](show command) Clamp 64-bit LIMIT and OFFSET before
narrowing in SHOW paging (#67008)
ed0a95d7005 is described below
commit ed0a95d70057f5ef516163457d175c2a432aee71
Author: Ambuj Upadhyay <[email protected]>
AuthorDate: Wed Aug 26 16:49:42 2026 +0530
[fix](show command) Clamp 64-bit LIMIT and OFFSET before narrowing in SHOW
paging (#67008)
### What problem does this PR solve?
Problem Summary: LIMIT and OFFSET reach the SHOW paging code as unbounded
64-bit
values. `limitClause` accepts a bare digit sequence, `LogicalPlanBuilder`
parses
it with `Long.parseLong` and checks only that it is non-negative, and
`LimitElement` stores both as `long`. Every paging site then narrowed them
to
`int` before clamping, so a value above `Integer.MAX_VALUE` wrapped.
Two failure modes on a 5-row result:
- `LIMIT 4294967296` -> `(int) (0 + 4294967296L)` is 0, the `endIndex >
size`
clamp does not fire, and `subList(0, 0)` returns an empty result. The user
gets
zero rows back with no error.
- `LIMIT 3000000000` -> `(int) 3000000000L` is -1294967296, which is not
greater
than size so it is not clamped; the `beginIndex > endIndex` branch then sets
`beginIndex` to the same negative value and `subList` throws
`IndexOutOfBoundsException`. `OFFSET 3000000000` reaches the same throw.
The same block was duplicated at eight sites across four proc dirs, backing
`SHOW PARTITIONS`, `SHOW ALTER TABLE COLUMN`, `SHOW ALTER TABLE ROLLUP` and
`SHOW BUILD INDEX`. `RollupProcDir` additionally lacked the `beginIndex
> endIndex` guard that its three siblings have, so it could call `subList`
with
`beginIndex > endIndex` and throw `IllegalArgumentException`.
`ShowCommand.applyLimit`, which pages `SHOW LOAD`, `SHOW RESOURCES` and
`SHOW PARTITIONS`, has the same defect in a different shape: `(limit +
offsetValue)`
is computed in long but can overflow to a negative value, which then
passes the `< showResult.size()` test and is narrowed for `subList`.
### What is changed and how does it work?
Added `LimitElement.applyTo(List)`, which owns both values already, and
computes
the window in long, saturating at `rows.size()` before narrowing:
- the offset is clamped into `[0, size]` first, so an out-of-range offset
yields
an empty window instead of a negative index;
- `begin + limit` is checked for a negative result, which is the signature
of the
long addition overflowing;
- with no limit set the window runs to the end of the list.
The eight duplicated blocks now delegate to it, which also gives
`RollupProcDir`
the guard it was missing. `ShowCommand.applyLimit` delegates too, keeping
its
existing `limit == -1` contract (no limit means return the list unchanged)
so
behaviour is identical apart from the overflow.
Net effect is 36 fewer lines and one tested implementation instead of eight
copies.
### Release note
Fix `SHOW PARTITIONS`, `SHOW ALTER TABLE`, `SHOW BUILD INDEX`, `SHOW LOAD`
and
`SHOW RESOURCES` returning an empty result or failing with
`IndexOutOfBoundsException` when LIMIT or OFFSET exceeds the 32-bit range.
---
.../org/apache/doris/analysis/LimitElement.java | 27 +++++++
.../doris/common/proc/BuildIndexProcDir.java | 17 +----
.../doris/common/proc/PartitionsProcDir.java | 24 +-----
.../apache/doris/common/proc/RollupProcDir.java | 17 +----
.../doris/common/proc/SchemaChangeProcDir.java | 17 +----
.../nereids/trees/plans/commands/ShowCommand.java | 16 ++--
.../apache/doris/analysis/LimitElementTest.java | 85 ++++++++++++++++++++++
7 files changed, 126 insertions(+), 77 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/analysis/LimitElement.java
b/fe/fe-core/src/main/java/org/apache/doris/analysis/LimitElement.java
index fd11ec028ce..10c70010c79 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/LimitElement.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/LimitElement.java
@@ -20,6 +20,8 @@
package org.apache.doris.analysis;
+import java.util.List;
+
/**
* Combination of limit and offset expressions.
*/
@@ -62,6 +64,31 @@ public class LimitElement {
return offset;
}
+ /**
+ * Returns the window of {@code rows} selected by this offset and limit.
+ *
+ * <p>Both values reach here as user supplied 64-bit integers, so the
range is computed in
+ * long and saturated at {@code rows.size()} before it is narrowed to int.
Narrowing first
+ * wraps: an offset or limit above {@link Integer#MAX_VALUE} can truncate
to zero and
+ * silently return an empty window, or truncate to a negative index and
make
+ * {@link List#subList} throw {@link IndexOutOfBoundsException}.
+ *
+ * <p>When no limit is set, the window runs from the offset to the end of
{@code rows}.
+ */
+ public <T> List<T> applyTo(List<T> rows) {
+ int size = rows.size();
+ long begin = Math.min(Math.max(offset, 0L), size);
+ long end = size;
+ if (hasLimit()) {
+ end = begin + limit;
+ // A negative sum means the long addition itself overflowed.
+ if (end < 0 || end > size) {
+ end = size;
+ }
+ }
+ return rows.subList((int) begin, (int) end);
+ }
+
public String toSql() {
if (limit == -1) {
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/common/proc/BuildIndexProcDir.java
b/fe/fe-core/src/main/java/org/apache/doris/common/proc/BuildIndexProcDir.java
index a0552af614d..46e0040eca5 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/common/proc/BuildIndexProcDir.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/common/proc/BuildIndexProcDir.java
@@ -250,15 +250,7 @@ public class BuildIndexProcDir implements ProcDirInterface
{
//limit
if (limitElement != null && limitElement.hasLimit()) {
- int beginIndex = (int) limitElement.getOffset();
- int endIndex = (int) (beginIndex + limitElement.getLimit());
- if (endIndex > jobInfos.size()) {
- endIndex = jobInfos.size();
- }
- if (beginIndex > endIndex) {
- beginIndex = endIndex;
- }
- jobInfos = jobInfos.subList(beginIndex, endIndex);
+ jobInfos = limitElement.applyTo(jobInfos);
}
BaseProcResult result = new BaseProcResult();
@@ -315,12 +307,7 @@ public class BuildIndexProcDir implements ProcDirInterface
{
//limit
if (limitElement != null && limitElement.hasLimit()) {
- int beginIndex = (int) limitElement.getOffset();
- int endIndex = (int) (beginIndex + limitElement.getLimit());
- if (endIndex > jobInfos.size()) {
- endIndex = jobInfos.size();
- }
- jobInfos = jobInfos.subList(beginIndex, endIndex);
+ jobInfos = limitElement.applyTo(jobInfos);
}
BaseProcResult result = new BaseProcResult();
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/common/proc/PartitionsProcDir.java
b/fe/fe-core/src/main/java/org/apache/doris/common/proc/PartitionsProcDir.java
index 049172f8b3a..1354e71bbfa 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/common/proc/PartitionsProcDir.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/common/proc/PartitionsProcDir.java
@@ -314,17 +314,7 @@ public class PartitionsProcDir implements ProcDirInterface
{
//limit
if (limitElement != null && limitElement.hasLimit()) {
- int beginIndex = (int) limitElement.getOffset();
- int endIndex = (int) (beginIndex + limitElement.getLimit());
- if (endIndex > filterPartitionInfos.size()) {
- endIndex = filterPartitionInfos.size();
- }
-
- // means that beginIndex is bigger than
filterPartitionInfos.size(), just return empty
- if (beginIndex > endIndex) {
- beginIndex = endIndex;
- }
- filterPartitionInfos = filterPartitionInfos.subList(beginIndex,
endIndex);
+ filterPartitionInfos = limitElement.applyTo(filterPartitionInfos);
}
return getBasicProcResult(filterPartitionInfos);
@@ -369,17 +359,7 @@ public class PartitionsProcDir implements ProcDirInterface
{
//limit
if (limitElement != null && limitElement.hasLimit()) {
- int beginIndex = (int) limitElement.getOffset();
- int endIndex = (int) (beginIndex + limitElement.getLimit());
- if (endIndex > filterPartitionInfos.size()) {
- endIndex = filterPartitionInfos.size();
- }
-
- // means that beginIndex is bigger than
filterPartitionInfos.size(), just return empty
- if (beginIndex > endIndex) {
- beginIndex = endIndex;
- }
- filterPartitionInfos = filterPartitionInfos.subList(beginIndex,
endIndex);
+ filterPartitionInfos = limitElement.applyTo(filterPartitionInfos);
}
return getBasicProcResult(filterPartitionInfos);
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/common/proc/RollupProcDir.java
b/fe/fe-core/src/main/java/org/apache/doris/common/proc/RollupProcDir.java
index 92639040b3c..e3112b252f0 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/common/proc/RollupProcDir.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/common/proc/RollupProcDir.java
@@ -145,12 +145,7 @@ public class RollupProcDir implements ProcDirInterface {
//limit
if (limitElement != null && limitElement.hasLimit()) {
- int beginIndex = (int) limitElement.getOffset();
- int endIndex = (int) (beginIndex + limitElement.getLimit());
- if (endIndex > jobInfos.size()) {
- endIndex = jobInfos.size();
- }
- jobInfos = jobInfos.subList(beginIndex, endIndex);
+ jobInfos = limitElement.applyTo(jobInfos);
}
BaseProcResult result = new BaseProcResult();
@@ -218,15 +213,7 @@ public class RollupProcDir implements ProcDirInterface {
//limit
if (limitElement != null && limitElement.hasLimit()) {
- int beginIndex = (int) limitElement.getOffset();
- int endIndex = (int) (beginIndex + limitElement.getLimit());
- if (endIndex > jobInfos.size()) {
- endIndex = jobInfos.size();
- }
- if (beginIndex > endIndex) {
- beginIndex = endIndex;
- }
- jobInfos = jobInfos.subList(beginIndex, endIndex);
+ jobInfos = limitElement.applyTo(jobInfos);
}
BaseProcResult result = new BaseProcResult();
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/common/proc/SchemaChangeProcDir.java
b/fe/fe-core/src/main/java/org/apache/doris/common/proc/SchemaChangeProcDir.java
index 5ecd772acef..e981ddbd091 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/common/proc/SchemaChangeProcDir.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/common/proc/SchemaChangeProcDir.java
@@ -264,15 +264,7 @@ public class SchemaChangeProcDir implements
ProcDirInterface {
//limit
if (limitElement != null && limitElement.hasLimit()) {
- int beginIndex = (int) limitElement.getOffset();
- int endIndex = (int) (beginIndex + limitElement.getLimit());
- if (endIndex > jobInfos.size()) {
- endIndex = jobInfos.size();
- }
- if (beginIndex > endIndex) {
- beginIndex = endIndex;
- }
- jobInfos = jobInfos.subList(beginIndex, endIndex);
+ jobInfos = limitElement.applyTo(jobInfos);
}
BaseProcResult result = new BaseProcResult();
@@ -329,12 +321,7 @@ public class SchemaChangeProcDir implements
ProcDirInterface {
//limit
if (limitElement != null && limitElement.hasLimit()) {
- int beginIndex = (int) limitElement.getOffset();
- int endIndex = (int) (beginIndex + limitElement.getLimit());
- if (endIndex > jobInfos.size()) {
- endIndex = jobInfos.size();
- }
- jobInfos = jobInfos.subList(beginIndex, endIndex);
+ jobInfos = limitElement.applyTo(jobInfos);
}
BaseProcResult result = new BaseProcResult();
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ShowCommand.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ShowCommand.java
index 62e9632b509..39f91cfa544 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ShowCommand.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ShowCommand.java
@@ -17,6 +17,7 @@
package org.apache.doris.nereids.trees.plans.commands;
+import org.apache.doris.analysis.LimitElement;
import org.apache.doris.analysis.RedirectStatus;
import org.apache.doris.analysis.StmtType;
import org.apache.doris.common.AnalysisException;
@@ -107,17 +108,12 @@ public abstract class ShowCommand extends Command
implements Redirect {
return Lists.newArrayList();
}
- long offsetValue = offset == -1L ? 0 : offset;
- if (offsetValue >= showResult.size()) {
- showResult = Lists.newArrayList();
- } else if (limit != -1L) {
- if ((limit + offsetValue) < showResult.size()) {
- showResult = showResult.subList((int) offsetValue, (int)
(limit + offsetValue));
- } else {
- showResult = showResult.subList((int) offsetValue,
showResult.size());
- }
+ if (limit == -1L) {
+ return showResult;
}
- return showResult;
+ // offset and limit are 64-bit; LimitElement.applyTo saturates the
window in long before
+ // narrowing, so a value above Integer.MAX_VALUE cannot wrap into a
bogus sub-list range.
+ return new LimitElement(offset == -1L ? 0 : offset,
limit).applyTo(showResult);
}
@Override
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/analysis/LimitElementTest.java
b/fe/fe-core/src/test/java/org/apache/doris/analysis/LimitElementTest.java
new file mode 100644
index 00000000000..a238d13bb49
--- /dev/null
+++ b/fe/fe-core/src/test/java/org/apache/doris/analysis/LimitElementTest.java
@@ -0,0 +1,85 @@
+// 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.analysis;
+
+import com.google.common.collect.Lists;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import java.util.List;
+
+public class LimitElementTest {
+
+ private static List<Integer> rows(int n) {
+ List<Integer> rows = Lists.newArrayList();
+ for (int i = 0; i < n; i++) {
+ rows.add(i);
+ }
+ return rows;
+ }
+
+ @Test
+ public void testWindowWithinIntRange() {
+ Assertions.assertEquals(Lists.newArrayList(0, 1, 2), new
LimitElement(0, 3).applyTo(rows(5)));
+ Assertions.assertEquals(Lists.newArrayList(2, 3), new LimitElement(2,
2).applyTo(rows(5)));
+ Assertions.assertEquals(Lists.newArrayList(3, 4), new LimitElement(3,
100).applyTo(rows(5)));
+ }
+
+ // A limit larger than Integer.MAX_VALUE must select every remaining row.
Narrowing the
+ // offset+limit sum to int first truncates 4294967296 to 0, which silently
returned nothing.
+ @Test
+ public void testLimitAboveIntMaxReturnsAllRows() {
+ Assertions.assertEquals(rows(5), new LimitElement(0,
4294967296L).applyTo(rows(5)));
+ Assertions.assertEquals(rows(5), new LimitElement(0,
Long.MAX_VALUE).applyTo(rows(5)));
+ }
+
+ // 3000000000 narrows to a negative int, which made subList throw
IndexOutOfBoundsException.
+ @Test
+ public void testLimitTruncatingToNegativeIntReturnsAllRows() {
+ Assertions.assertEquals(rows(5), new LimitElement(0,
3000000000L).applyTo(rows(5)));
+ }
+
+ // The offset+limit addition itself can overflow long; the window must
still be clamped.
+ @Test
+ public void testOffsetPlusLimitOverflowIsClamped() {
+ Assertions.assertEquals(Lists.newArrayList(1, 2, 3, 4),
+ new LimitElement(1, Long.MAX_VALUE).applyTo(rows(5)));
+ }
+
+ // An offset past the end selects nothing, whatever its magnitude, and
never throws.
+ @Test
+ public void testOffsetBeyondEndReturnsEmpty() {
+ Assertions.assertTrue(new LimitElement(5,
10).applyTo(rows(5)).isEmpty());
+ Assertions.assertTrue(new LimitElement(3000000000L,
10).applyTo(rows(5)).isEmpty());
+ Assertions.assertTrue(new LimitElement(Long.MAX_VALUE,
10).applyTo(rows(5)).isEmpty());
+ }
+
+ // Without a limit the window runs from the offset to the end.
+ @Test
+ public void testNoLimitRunsToEnd() {
+ LimitElement noLimit = new LimitElement(2, -1);
+ Assertions.assertFalse(noLimit.hasLimit());
+ Assertions.assertEquals(Lists.newArrayList(2, 3, 4),
noLimit.applyTo(rows(5)));
+ }
+
+ @Test
+ public void testEmptyInput() {
+ Assertions.assertTrue(new LimitElement(0,
10).applyTo(rows(0)).isEmpty());
+ Assertions.assertTrue(new LimitElement(3000000000L,
10).applyTo(rows(0)).isEmpty());
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
