This is an automated email from the ASF dual-hosted git repository.
morningman 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 33a60b26ecc [fix](show) apply ORDER BY and LIMIT correctly to SHOW
TABLETS (#65871) (#66116)
33a60b26ecc is described below
commit 33a60b26ecc084ea63975cbca62d7911ddd41dfe
Author: SudharsanK2308 <[email protected]>
AuthorDate: Tue Aug 11 09:24:30 2026 +0530
[fix](show) apply ORDER BY and LIMIT correctly to SHOW TABLETS (#65871)
(#66116)
### What problem does this PR solve?
Issue Number: close #65871
Related PR: #xxx
Problem Summary:
`SHOW TABLETS FROM tbl ORDER BY <col> LIMIT n` returns the wrong rows.
While walking the partitions and materialized indexes of the table, the
scan stops as soon as
`offset + limit` rows have been collected, and the ORDER BY is applied
*after* that early exit.
The sort therefore only ever sees the prefix of the tablet set that
happened to be collected
first, so
```sql
SHOW TABLETS FROM tbl ORDER BY LocalDataSize DESC LIMIT 10;
```
returns the 10 largest tablets **of the first partitions/indexes that
were scanned**, not the 10
largest tablets of the table.
This PR makes the early exit conditional on the absence of an explicit
ORDER BY:
- **With ORDER BY**: every tablet of the requested partitions is
collected first, then sorted,
then truncated to `limit` rows, and OFFSET is applied on that sorted
result. The answer is now
the globally correct top-N.
- **Without ORDER BY**: unchanged. The scan still stops as soon as
enough rows are gathered, so
`LIMIT n` still returns a prefix of the scan, ordered by `(TabletId,
ReplicaId)` among itself.
The cheap path is deliberately kept for the common `SHOW TABLETS FROM
tbl LIMIT n` on tables
with a large number of tablets.
Two smaller issues fixed along the way:
- `sizeLimit` was a `long` narrowed by a plain `(int)` cast before
`subList()`. A large limit
(e.g. `LIMIT 3000000000`) wrapped around to a negative value and made
the statement fail with
`IndexOutOfBoundsException`. The value is now clamped to
`Integer.MAX_VALUE`.
- The "sort, then keep the first N rows" step is extracted into
`org.apache.doris.common.util.SortAndLimit`, a small helper for the
`List<List<Comparable>>`
rows used by the SHOW / proc-dir style commands. It copies before
sorting, so it never mutates
the list handed in by the caller.
Changed files:
- `common/util/SortAndLimit.java` (new): sort-then-truncate helper.
- `nereids/trees/plans/commands/ShowTabletsFromTableCommand.java`: skip
the early exit when
ORDER BY is given; always sort, then truncate, then apply OFFSET.
- `common/util/SortAndLimitTest.java` (new): unit tests for the helper.
- `regression-test/suites/show_p0/test_show_tablet.groovy`: coverage for
ORDER BY / LIMIT /
OFFSET on a 3-partition, 3-bucket table.
---
.../org/apache/doris/common/util/SortAndLimit.java | 57 ++++++++++++++++
.../commands/ShowTabletsFromTableCommand.java | 67 ++++++++++--------
.../apache/doris/common/util/SortAndLimitTest.java | 79 ++++++++++++++++++++++
.../suites/show_p0/test_show_tablet.groovy | 69 +++++++++++++++++++
4 files changed, 242 insertions(+), 30 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/common/util/SortAndLimit.java
b/fe/fe-core/src/main/java/org/apache/doris/common/util/SortAndLimit.java
new file mode 100644
index 00000000000..75fa9bc19c1
--- /dev/null
+++ b/fe/fe-core/src/main/java/org/apache/doris/common/util/SortAndLimit.java
@@ -0,0 +1,57 @@
+// 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.common.util;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+
+/**
+ * Utility for the common "sort, then truncate to the first N rows" pattern of
the SHOW/proc-dir
+ * style commands that operate on {@code List<List<Comparable>>} rows (see
{@link ListComparator}
+ * and {@link OrderByPair}).
+ */
+public class SortAndLimit {
+
+ private SortAndLimit() {
+ }
+
+ /**
+ * Sorts {@code rows} using {@code comparator} and returns a new list
truncated to the first
+ * {@code sizeLimit} elements.
+ *
+ * <p>This method does NOT mutate the {@code rows} list passed in: it
first copies the input
+ * into a new, mutable list and sorts that copy in place, so an immutable
input list can be
+ * passed safely and the caller's original list/order is left untouched.
+ *
+ * @param rows the rows to sort; not modified by this call
+ * @param comparator the comparator defining the sort order
+ * @param sizeLimit the maximum number of rows to keep, counted from the
start of the sorted
+ * result; {@link Optional#empty()} means "no limit"
(return every row)
+ * @return a new list, sorted by {@code comparator} and truncated to at
most {@code sizeLimit}
+ * elements
+ */
+ public static List<List<Comparable>> sortAndLimit(List<List<Comparable>>
rows,
+ ListComparator<List<Comparable>> comparator, Optional<Integer>
sizeLimit) {
+ List<List<Comparable>> sorted = new ArrayList<>(rows);
+ sorted.sort(comparator);
+
+ int limit = sizeLimit.orElse(sorted.size());
+ return new ArrayList<>(sorted.subList(0, Math.min(limit,
sorted.size())));
+ }
+}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ShowTabletsFromTableCommand.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ShowTabletsFromTableCommand.java
index 79e7413ee3d..6c6ffca87ab 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ShowTabletsFromTableCommand.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ShowTabletsFromTableCommand.java
@@ -36,6 +36,7 @@ import org.apache.doris.common.UserException;
import org.apache.doris.common.proc.TabletsProcDir;
import org.apache.doris.common.util.ListComparator;
import org.apache.doris.common.util.OrderByPair;
+import org.apache.doris.common.util.SortAndLimit;
import org.apache.doris.common.util.Util;
import org.apache.doris.mysql.privilege.PrivPredicate;
import org.apache.doris.nereids.analyzer.UnboundSlot;
@@ -57,9 +58,9 @@ import com.google.common.collect.Lists;
import java.util.ArrayList;
import java.util.Collection;
-import java.util.Collections;
import java.util.List;
import java.util.Locale;
+import java.util.Optional;
/**
* ShowTabletsFromTableCommand
@@ -201,13 +202,16 @@ public class ShowTabletsFromTableCommand extends
ShowCommand {
OlapTable olapTable =
db.getOlapTableOrAnalysisException(dbTableName.getTbl());
olapTable.readLock();
try {
- long sizeLimit = -1;
- if (offset > 0 && limit > 0) {
- sizeLimit = offset + limit;
- } else if (limit > 0) {
- sizeLimit = limit;
+ // The parser passes limit = 0 when the statement carries no LIMIT
clause
+ // (see LogicalPlanBuilder#visitShowTabletsFromTable), so only a
positive limit
+ // bounds the result. sizeLimit is how many sorted rows have to be
kept: the LIMIT
+ // rows plus the OFFSET rows that are skipped afterwards.
+ Optional<Integer> sizeLimit = Optional.empty();
+ if (limit > 0) {
+ long capped = limit + Math.max(offset, 0);
+ sizeLimit = Optional.of((int) Math.min(capped,
Integer.MAX_VALUE));
}
- boolean stop = false;
+
Collection<Partition> partitions = new ArrayList<Partition>();
if (partitionNames != null) {
List<String> paNames = partitionNames.getPartitionNames();
@@ -223,6 +227,12 @@ public class ShowTabletsFromTableCommand extends
ShowCommand {
} else {
partitions = olapTable.getPartitions();
}
+ // With an explicit ORDER BY every tablet has to be collected
before the result can be
+ // truncated, otherwise the sort only sees an arbitrary prefix of
the scan and returns
+ // the wrong rows -- the bug reported in #65871. Without ORDER BY
the scan still stops
+ // as soon as enough rows are gathered, as it did before: LIMIT
then returns a prefix
+ // of the scan, ordered by (tabletId, replicaId) among itself.
+ boolean stop = false;
List<List<Comparable>> tabletInfos = new ArrayList<>();
for (Partition partition : partitions) {
if (stop) {
@@ -232,37 +242,34 @@ public class ShowTabletsFromTableCommand extends
ShowCommand {
TabletsProcDir procDir = new TabletsProcDir(olapTable,
index);
tabletInfos.addAll(procDir.fetchComparableResult(
version, backendId, replicaState));
- if (sizeLimit > -1 && tabletInfos.size() >= sizeLimit) {
+ if (orderByPairs == null && sizeLimit.isPresent() &&
tabletInfos.size() >= sizeLimit.get()) {
stop = true;
break;
}
}
}
- if (offset >= tabletInfos.size()) {
- tabletInfos.clear();
+
+ ListComparator<List<Comparable>> comparator;
+ if (orderByPairs != null) {
+ // order by the keys given by the user
+ OrderByPair[] orderByPairArr = new
OrderByPair[orderByPairs.size()];
+ comparator = new
ListComparator<>(orderByPairs.toArray(orderByPairArr));
} else {
- // order by
- ListComparator<List<Comparable>> comparator = null;
- if (orderByPairs != null) {
- OrderByPair[] orderByPairArr = new
OrderByPair[orderByPairs.size()];
- comparator = new
ListComparator<>(orderByPairs.toArray(orderByPairArr));
- } else {
- // order by tabletId, replicaId
- comparator = new ListComparator<>(0, 1);
- }
- Collections.sort(tabletInfos, comparator);
- if (sizeLimit > -1) {
- tabletInfos = tabletInfos.subList((int) offset,
- Math.min((int) sizeLimit, tabletInfos.size()));
- }
+ // order by tabletId, replicaId
+ comparator = new ListComparator<>(0, 1);
+ }
+ List<List<Comparable>> orderedTabletInfos =
SortAndLimit.sortAndLimit(tabletInfos, comparator, sizeLimit);
- for (List<Comparable> tabletInfo : tabletInfos) {
- List<String> oneTablet = new
ArrayList<String>(tabletInfo.size());
- for (Comparable column : tabletInfo) {
- oneTablet.add(column.toString());
- }
- rows.add(oneTablet);
+ // If offset is beyond the end of the result, subList yields an
empty list and no row
+ // is returned.
+ int resultOffset = (int) Math.min(offset,
orderedTabletInfos.size());
+ for (List<Comparable> tabletInfo
+ : orderedTabletInfos.subList(resultOffset,
orderedTabletInfos.size())) {
+ List<String> oneTablet = new
ArrayList<String>(tabletInfo.size());
+ for (Comparable column : tabletInfo) {
+ oneTablet.add(column.toString());
}
+ rows.add(oneTablet);
}
} finally {
olapTable.readUnlock();
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/common/util/SortAndLimitTest.java
b/fe/fe-core/src/test/java/org/apache/doris/common/util/SortAndLimitTest.java
new file mode 100644
index 00000000000..0b72552d72e
--- /dev/null
+++
b/fe/fe-core/src/test/java/org/apache/doris/common/util/SortAndLimitTest.java
@@ -0,0 +1,79 @@
+// 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.common.util;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Lists;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.List;
+import java.util.Optional;
+
+public class SortAndLimitTest {
+
+ private static final ListComparator<List<Comparable>> BY_FIRST_COLUMN =
new ListComparator<>(0);
+
+ private static List<List<Comparable>> rows(Comparable...
firstColumnValues) {
+ List<List<Comparable>> rows = Lists.newArrayList();
+ for (Comparable value : firstColumnValues) {
+ rows.add(Lists.newArrayList(value));
+ }
+ return rows;
+ }
+
+ private static List<Comparable> firstColumnOf(List<List<Comparable>> rows)
{
+ List<Comparable> values = Lists.newArrayList();
+ for (List<Comparable> row : rows) {
+ values.add(row.get(0));
+ }
+ return values;
+ }
+
+ @Test
+ public void testEmptyLimitKeepsEveryRow() {
+ List<List<Comparable>> sorted = SortAndLimit.sortAndLimit(rows(3L, 1L,
2L), BY_FIRST_COLUMN,
+ Optional.empty());
+ Assert.assertEquals(Lists.newArrayList(1L, 2L, 3L),
firstColumnOf(sorted));
+ }
+
+ @Test
+ public void testLimitAppliesToTheSortedResult() {
+ // the two smallest values, not the first two rows of the input
+ List<List<Comparable>> sorted = SortAndLimit.sortAndLimit(rows(3L, 1L,
2L), BY_FIRST_COLUMN,
+ Optional.of(2));
+ Assert.assertEquals(Lists.newArrayList(1L, 2L), firstColumnOf(sorted));
+ }
+
+ @Test
+ public void testLimitLargerThanInputIsClamped() {
+ List<List<Comparable>> sorted = SortAndLimit.sortAndLimit(rows(3L,
1L), BY_FIRST_COLUMN,
+ Optional.of(100));
+ Assert.assertEquals(Lists.newArrayList(1L, 3L), firstColumnOf(sorted));
+ }
+
+ @Test
+ public void testInputIsNotModified() {
+ List<List<Comparable>> input = ImmutableList.<List<Comparable>>of(
+ ImmutableList.<Comparable>of(3L),
+ ImmutableList.<Comparable>of(1L));
+ List<List<Comparable>> sorted = SortAndLimit.sortAndLimit(input,
BY_FIRST_COLUMN, Optional.of(1));
+ Assert.assertEquals(Lists.newArrayList(1L), firstColumnOf(sorted));
+ Assert.assertEquals(Lists.newArrayList(3L, 1L), firstColumnOf(input));
+ }
+}
diff --git a/regression-test/suites/show_p0/test_show_tablet.groovy
b/regression-test/suites/show_p0/test_show_tablet.groovy
index abe54d7e93f..51e5e4fae46 100644
--- a/regression-test/suites/show_p0/test_show_tablet.groovy
+++ b/regression-test/suites/show_p0/test_show_tablet.groovy
@@ -59,4 +59,73 @@ suite("test_show_tablet") {
} else {
assertTrue(1 == 2)
}
+
+ // An explicit ORDER BY must be applied to the whole tablet set of the
table, not to the
+ // prefix that happens to be collected first while walking partitions and
indexes.
+ sql """drop table if exists show_tablets_multi_part_t;"""
+ sql """create table show_tablets_multi_part_t (
+ id INT,
+ username VARCHAR(20)
+ )
+ DUPLICATE KEY(id)
+ PARTITION BY RANGE(id) (
+ PARTITION p1 VALUES LESS THAN (10),
+ PARTITION p2 VALUES LESS THAN (20),
+ PARTITION p3 VALUES LESS THAN (30)
+ )
+ DISTRIBUTED BY HASH(id) BUCKETS 3
+ PROPERTIES (
+ "replication_num" = "1"
+ );"""
+
+ def allTablets = sql """SHOW TABLETS FROM show_tablets_multi_part_t"""
+ logger.info("all tablets: " + allTablets.toString())
+ // 3 partitions * 3 buckets, one row per replica
+ assertTrue(allTablets.size() >= 9)
+
+ def allIds = allTablets.collect { it[0] as long }
+ def ascIds = new ArrayList(allIds)
+ Collections.sort(ascIds)
+ def descIds = new ArrayList(ascIds)
+ Collections.reverse(descIds)
+
+ // without ORDER BY the rows come back ordered by (TabletId, ReplicaId)
+ assertEquals(ascIds, allIds)
+
+ // ORDER BY without LIMIT returns every tablet
+ res = sql """SHOW TABLETS FROM show_tablets_multi_part_t ORDER BY TabletId
DESC"""
+ assertEquals(descIds, res.collect { it[0] as long })
+
+ // ORDER BY ... LIMIT returns the globally largest tablet ids, not the
largest ones
+ // of the first partition scanned
+ res = sql """SHOW TABLETS FROM show_tablets_multi_part_t ORDER BY TabletId
DESC LIMIT 3"""
+ assertEquals(descIds.subList(0, 3), res.collect { it[0] as long })
+
+ // OFFSET is applied after sorting
+ res = sql """SHOW TABLETS FROM show_tablets_multi_part_t ORDER BY TabletId
DESC LIMIT 2, 3"""
+ assertEquals(descIds.subList(2, 5), res.collect { it[0] as long })
+
+ // Without ORDER BY the scan stops as soon as enough rows are gathered, so
LIMIT returns a
+ // prefix of the scan rather than the globally smallest tablet ids. Only
the row count and
+ // the ordering inside the returned prefix are guaranteed.
+ def assertPrefixOfTable = { rows, expectedSize ->
+ assertEquals(expectedSize, rows.size())
+ def ids = rows.collect { it[0] as long }
+ def sortedIds = new ArrayList(ids)
+ Collections.sort(sortedIds)
+ assertEquals(sortedIds, ids)
+ assertTrue(allIds.containsAll(ids))
+ }
+
+ res = sql """SHOW TABLETS FROM show_tablets_multi_part_t LIMIT 3"""
+ assertPrefixOfTable(res, 3)
+
+ res = sql """SHOW TABLETS FROM show_tablets_multi_part_t LIMIT 2, 3"""
+ assertPrefixOfTable(res, 3)
+
+ // an offset past the end of the result yields no row
+ res = sql """SHOW TABLETS FROM show_tablets_multi_part_t LIMIT
${allTablets.size()}, 3"""
+ assertTrue(res.isEmpty())
+
+ sql """drop table if exists show_tablets_multi_part_t;"""
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
