This is an automated email from the ASF dual-hosted git repository.
csun5285 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 a43d0d879a8 [fix](be) Do not push storage TopN to a MOR table read as
DUP (#68804)
a43d0d879a8 is described below
commit a43d0d879a8521c2330a81b047dbe245e78bee73
Author: Chenyang Sun <[email protected]>
AuthorDate: Wed Oct 14 09:34:23 2026 +0800
[fix](be) Do not push storage TopN to a MOR table read as DUP (#68804)
Storage TopN does not support MOR tables read as DUP
(`read_mor_as_dup_tables`), so the scanner no longer pushes it down for
them; doing so aborted the BE.
---
be/src/exec/scan/olap_scanner.cpp | 10 ++--
be/src/storage/tablet/tablet_reader.cpp | 10 ++++
be/test/storage/tablet_reader_test.cpp | 61 ++++++++++++++++++++++
.../test_read_mor_as_dup_key_topn.out | 35 +++++++++++++
.../test_read_mor_as_dup_key_topn.groovy | 54 +++++++++++++++++++
5 files changed, 167 insertions(+), 3 deletions(-)
diff --git a/be/src/exec/scan/olap_scanner.cpp
b/be/src/exec/scan/olap_scanner.cpp
index 1762ee1c154..c14829dd7ea 100644
--- a/be/src/exec/scan/olap_scanner.cpp
+++ b/be/src/exec/scan/olap_scanner.cpp
@@ -475,6 +475,9 @@ Status OlapScanner::_init_tablet_reader_params(
const bool no_runtime_filters = _total_rf_num == 0;
const bool segment_limit_enabled =
_state->enable_segment_limit_pushdown();
const bool storage_no_merge =
olap_scan_local_state->_storage_no_merge();
+ // Storage TopN compares key columns that TabletReader prepares only
for DUP and
+ // merge-on-write tablets, so a MOR table read as DUP cannot use it.
+ const bool storage_topn_supported = storage_no_merge &&
!read_mor_as_dup;
if (_limit > 0 && no_runtime_filters && segment_limit_enabled &&
storage_no_merge) {
for (const auto& conjunct : _conjuncts) {
@@ -486,9 +489,10 @@ Status OlapScanner::_init_tablet_reader_params(
// Segment LIMIT has only two legal states: completely disabled, or
enabled after every
// row-filtering conjunct has become a storage predicate or
SegmentIterator common expr.
- const bool can_push_down_segment_limit = _limit > 0 &&
no_runtime_filters &&
- _conjuncts.empty() &&
segment_limit_enabled &&
- storage_no_merge;
+ // A key TopN also needs storage TopN support, otherwise nothing is
pushed.
+ const bool can_push_down_segment_limit =
+ _limit > 0 && no_runtime_filters && _conjuncts.empty() &&
segment_limit_enabled &&
+ storage_no_merge && (!has_key_topn || storage_topn_supported);
if (can_push_down_segment_limit) {
if (has_key_topn) {
_tablet_reader_params.read_orderby_key = true;
diff --git a/be/src/storage/tablet/tablet_reader.cpp
b/be/src/storage/tablet/tablet_reader.cpp
index db2744a3f9f..295c44650d8 100644
--- a/be/src/storage/tablet/tablet_reader.cpp
+++ b/be/src/storage/tablet/tablet_reader.cpp
@@ -379,6 +379,16 @@ Status TabletReader::_init_orderby_keys_param(const
ReaderParams& read_params) {
}
}
+ // Storage TopN sorts rows by these columns, and only DUP and
merge-on-write tablets set
+ // them up. Without them, fail the scan here instead of aborting later in
VCollectIterator.
+ if (read_params.read_orderby_key_limit > 0 &&
_orderby_key_columns.empty()) {
+ return Status::Error<ErrorCode::INTERNAL_ERROR>(
+ "storage TopN needs order-by key columns, but none are set up:
keys_type={}, "
+ "enable_unique_key_merge_on_write={},
read_orderby_key_num_prefix_columns={}",
+ _tablet_schema->keys_type(),
_tablet->enable_unique_key_merge_on_write(),
+ read_params.read_orderby_key_num_prefix_columns);
+ }
+
return Status::OK();
}
diff --git a/be/test/storage/tablet_reader_test.cpp
b/be/test/storage/tablet_reader_test.cpp
index 1d43acedd9d..22cfdf70177 100644
--- a/be/test/storage/tablet_reader_test.cpp
+++ b/be/test/storage/tablet_reader_test.cpp
@@ -31,6 +31,9 @@
#include "storage/delete/delete_handler.h"
#include "storage/rowset/rowset_meta.h"
#include "storage/schema.h"
+#include "storage/storage_engine.h"
+#include "storage/tablet/tablet.h"
+#include "storage/tablet/tablet_meta.h"
#include "storage/tablet/tablet_schema.h"
namespace doris {
@@ -70,6 +73,25 @@ protected:
ASSERT_TRUE(handler.init({rs_meta}, /*version=*/100, *read_schema,
&dropped_columns).ok());
(*read_schema)->append_dropped_columns(std::move(dropped_columns));
}
+
+ // Give `reader` a tablet with the given keys type. Its only key column is
k1.
+ static void init_tablet(StorageEngine& engine, TabletReader& reader,
KeysType keys_type,
+ bool enable_mow) {
+ auto tablet_meta = std::make_shared<TabletMeta>();
+ tablet_meta->set_enable_unique_key_merge_on_write(enable_mow);
+ reader._tablet = std::make_shared<Tablet>(engine,
std::move(tablet_meta), nullptr);
+ reader._tablet_schema = create_schema({{"k1", 10}, {"v1", 12}},
keys_type);
+ reader._read_schema =
std::make_shared<ReadSchema>(reader._tablet_schema->columns());
+ }
+
+ // Set up the order-by key columns for a storage TopN that sorts by the
first
+ // `num_prefix_columns` key columns.
+ static Status init_storage_topn(TabletReader& reader, size_t
num_prefix_columns) {
+ TabletReader::ReaderParams params;
+ params.read_orderby_key_num_prefix_columns = num_prefix_columns;
+ params.read_orderby_key_limit = 10;
+ return reader._init_orderby_keys_param(params);
+ }
};
// The delete columns (resolved by the delete handler to read-schema ordinals)
are mapped to their
@@ -211,4 +233,43 @@ TEST_F(TabletReaderTest,
delete_predicate_keeps_dropped_same_name_column_identit
EXPECT_EQ(1, delete_zone_maps.count(2));
}
}
+
+// DUP and merge-on-write tablets give storage TopN the leading key columns to
sort by.
+TEST_F(TabletReaderTest, storage_topn_sorts_by_leading_key_columns) {
+ StorageEngine engine({});
+ TabletReader dup_reader;
+ init_tablet(engine, dup_reader, KeysType::DUP_KEYS, /*enable_mow=*/false);
+ auto st = init_storage_topn(dup_reader, 1);
+ ASSERT_TRUE(st.ok()) << st;
+ EXPECT_EQ(std::vector<uint32_t>({0}), dup_reader._orderby_key_columns);
+
+ TabletReader mow_reader;
+ init_tablet(engine, mow_reader, KeysType::UNIQUE_KEYS,
/*enable_mow=*/true);
+ st = init_storage_topn(mow_reader, 1);
+ ASSERT_TRUE(st.ok()) << st;
+ EXPECT_EQ(std::vector<uint32_t>({0}), mow_reader._orderby_key_columns);
+}
+
+// Storage TopN without order-by key columns fails at init, instead of
aborting the BE
+// later in VCollectIterator.
+TEST_F(TabletReaderTest, storage_topn_without_orderby_key_columns_fails) {
+ StorageEngine engine({});
+ // A merge-on-read tablet, such as one read as DUP, sets up no order-by
key columns.
+ TabletReader mor_reader;
+ init_tablet(engine, mor_reader, KeysType::UNIQUE_KEYS,
/*enable_mow=*/false);
+ auto st = init_storage_topn(mor_reader, 1);
+ EXPECT_TRUE(st.is<ErrorCode::INTERNAL_ERROR>()) << st;
+
+ // An empty key prefix leaves nothing to sort by.
+ TabletReader dup_reader;
+ init_tablet(engine, dup_reader, KeysType::DUP_KEYS, /*enable_mow=*/false);
+ st = init_storage_topn(dup_reader, 0);
+ EXPECT_TRUE(st.is<ErrorCode::INTERNAL_ERROR>()) << st;
+
+ // Without TopN, a merge-on-read tablet needs no order-by key columns.
+ TabletReader plain_reader;
+ init_tablet(engine, plain_reader, KeysType::UNIQUE_KEYS,
/*enable_mow=*/false);
+ st = plain_reader._init_orderby_keys_param(TabletReader::ReaderParams {});
+ EXPECT_TRUE(st.ok()) << st;
+}
} // namespace doris
diff --git
a/regression-test/data/unique_with_mor_p0/test_read_mor_as_dup_key_topn.out
b/regression-test/data/unique_with_mor_p0/test_read_mor_as_dup_key_topn.out
new file mode 100644
index 00000000000..2955af22da3
--- /dev/null
+++ b/regression-test/data/unique_with_mor_p0/test_read_mor_as_dup_key_topn.out
@@ -0,0 +1,35 @@
+-- This file is automatically generated. You should know what you did if you
want to edit this
+-- !merged --
+1 1 12
+2 2 20
+3 3 30
+
+-- !dup_asc --
+1 1 10
+1 1 11
+1 1 12
+2 2 20
+
+-- !dup_desc --
+7 7 70
+6 6 60
+5 5 50
+
+-- !dup_prefix --
+1 1 10
+1 1 11
+1 1 12
+2 2 20
+
+-- !dup_filter --
+1 1 11
+1 1 12
+2 2 20
+
+-- !dup_offset --
+2 2 20
+3 3 30
+
+-- !dup_limit --
+5
+
diff --git
a/regression-test/suites/unique_with_mor_p0/test_read_mor_as_dup_key_topn.groovy
b/regression-test/suites/unique_with_mor_p0/test_read_mor_as_dup_key_topn.groovy
new file mode 100644
index 00000000000..10bde8f280a
--- /dev/null
+++
b/regression-test/suites/unique_with_mor_p0/test_read_mor_as_dup_key_topn.groovy
@@ -0,0 +1,54 @@
+// 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.
+
+// ORDER BY a key prefix with LIMIT on a MOR table listed in
read_mor_as_dup_tables.
+// The table is read as DUP, so every version of a key is a row and the TopN
sees all of them.
+suite("test_read_mor_as_dup_key_topn") {
+ sql "DROP TABLE IF EXISTS test_read_mor_as_dup_key_topn"
+ sql """
+ CREATE TABLE test_read_mor_as_dup_key_topn (
+ k1 INT NOT NULL,
+ k2 INT NOT NULL,
+ v INT NULL
+ ) ENGINE=OLAP
+ UNIQUE KEY(k1, k2)
+ DISTRIBUTED BY HASH(k1) BUCKETS 1
+ PROPERTIES (
+ "enable_unique_key_merge_on_write" = "false",
+ "disable_auto_compaction" = "true",
+ "replication_num" = "1"
+ )
+ """
+ // Three loads make three overlapping rowsets; key (1, 1) has three
versions.
+ sql "INSERT INTO test_read_mor_as_dup_key_topn VALUES (1, 1, 10), (2, 2,
20), (5, 5, 50)"
+ sql "INSERT INTO test_read_mor_as_dup_key_topn VALUES (1, 1, 11), (3, 3,
30), (6, 6, 60)"
+ sql "INSERT INTO test_read_mor_as_dup_key_topn VALUES (1, 1, 12), (4, 4,
40), (7, 7, 70)"
+
+ // Merged read: one row per key.
+ qt_merged "SELECT * FROM test_read_mor_as_dup_key_topn ORDER BY k1, k2
LIMIT 3"
+
+ sql "SET read_mor_as_dup_tables = 'test_read_mor_as_dup_key_topn'"
+ // The crash needs segment LIMIT pushdown, which fuzzy sessions may turn
off.
+ sql "SET enable_segment_limit_pushdown = true"
+ order_qt_dup_asc "SELECT * FROM test_read_mor_as_dup_key_topn ORDER BY k1,
k2 LIMIT 4"
+ qt_dup_desc "SELECT * FROM test_read_mor_as_dup_key_topn ORDER BY k1 DESC,
k2 DESC LIMIT 3"
+ order_qt_dup_prefix "SELECT * FROM test_read_mor_as_dup_key_topn ORDER BY
k1 LIMIT 4"
+ order_qt_dup_filter "SELECT * FROM test_read_mor_as_dup_key_topn WHERE v >
10 ORDER BY k1, k2 LIMIT 3"
+ qt_dup_offset "SELECT * FROM test_read_mor_as_dup_key_topn ORDER BY k1, k2
LIMIT 2 OFFSET 3"
+ // A plain LIMIT without ORDER BY still works on the table read as DUP.
+ order_qt_dup_limit "SELECT count(*) FROM (SELECT * FROM
test_read_mor_as_dup_key_topn LIMIT 5) t"
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
