github-actions[bot] commented on code in PR #67563:
URL: https://github.com/apache/doris/pull/67563#discussion_r4217163463
##########
be/test/exec/rowid_fetcher_test.cpp:
##########
@@ -256,4 +286,141 @@ TEST_F(RowIdStorageReaderTest,
ExternalFetchPartitionSlotsPreserveHivePositionMa
}
}
+TEST_F(RowIdStorageReaderTest, SameSourceColumnSharesKey) {
+ // The bug case: one physical column projected twice must dedup onto one
scan column.
+ const SlotDescriptor first = make_slot({});
+ const SlotDescriptor second = make_slot({});
+ EXPECT_EQ(key_of(first, 3), key_of(second, 3));
+}
+
+TEST_F(RowIdStorageReaderTest, ColumnIndexSeparatesKeys) {
+ const SlotDescriptor slot = make_slot({});
+ EXPECT_NE(key_of(slot, 3), key_of(slot, 4));
+}
+
+TEST_F(RowIdStorageReaderTest, ColumnNameSeparatesKeys) {
+ EXPECT_NE(key_of(make_slot({.col_name = "a"}), 0),
key_of(make_slot({.col_name = "b"}), 0));
+}
+
+TEST_F(RowIdStorageReaderTest, UniqueIdSeparatesKeys) {
+ EXPECT_NE(key_of(make_slot({.col_unique_id = 1}), 0),
+ key_of(make_slot({.col_unique_id = 2}), 0));
+}
+
+TEST_F(RowIdStorageReaderTest, NameAndIndexBoundaryIsNotAmbiguous) {
+ // Without length prefixes, "a" + idx 12 and "a1" + idx 2 both flatten to
"a12".
+ EXPECT_NE(key_of(make_slot({.col_name = "a"}), 12),
key_of(make_slot({.col_name = "a1"}), 2));
+}
+
+TEST_F(RowIdStorageReaderTest, PathComponentBoundaryIsNotAmbiguous) {
+ // The concatenation hazard the length prefix exists for: ["a", "b"] and
["a:b"] are
+ // different nested columns but share the naive ':'-joined spelling.
+ EXPECT_NE(key_of(make_slot({.column_paths = {"a", "b"}}), 0),
+ key_of(make_slot({.column_paths = {"a:b"}}), 0));
+}
+
+TEST_F(RowIdStorageReaderTest, EmptyPathIsNotTheSameAsNoPath) {
+ EXPECT_NE(key_of(make_slot({.column_paths = {}}), 0),
+ key_of(make_slot({.column_paths = {""}}), 0));
+}
+
+TEST_F(RowIdStorageReaderTest, PathOrderMatters) {
+ EXPECT_NE(key_of(make_slot({.column_paths = {"a", "b"}}), 0),
+ key_of(make_slot({.column_paths = {"b", "a"}}), 0));
+}
+
+TEST_F(RowIdStorageReaderTest, EqualPathsShareKey) {
+ EXPECT_EQ(key_of(make_slot({.column_paths = {"a", "b"}}), 0),
+ key_of(make_slot({.column_paths = {"a", "b"}}), 0));
+}
+
+TEST_F(RowIdStorageReaderTest, AccessPathSeparatesKeys) {
+ EXPECT_NE(key_of(make_slot({.access_paths = {data_path({"a"})}}), 0),
+ key_of(make_slot({.access_paths = {data_path({"b"})}}), 0));
+}
+
+TEST_F(RowIdStorageReaderTest, AbsentAccessPathIsNotAnEmptyOne) {
+ // The presence bit: an unset data_access_path must not collide with one
that is set
+ // but carries no components.
+ EXPECT_NE(key_of(make_slot({.access_paths = {bare_path()}}), 0),
+ key_of(make_slot({.access_paths = {data_path({})}}), 0));
+}
+
+TEST_F(RowIdStorageReaderTest, AccessPathCountSeparatesKeys) {
+ EXPECT_NE(key_of(make_slot({.access_paths = {data_path({"a"})}}), 0),
+ key_of(make_slot({.access_paths = {data_path({"a"}),
data_path({"b"})}}), 0));
+}
+
+// Runs every submitted task on the submitting thread. The point of these
cases is which
+// status reaches the caller, not the threading, and inline execution keeps
them
+// deterministic.
+class InlineScanScheduler : public ScannerScheduler {
+public:
+ Status start(int, int, int, int) override { return Status::OK(); }
+ void stop() override {}
+ Status submit_scan_task(SimplifiedScanTask scan_task) override {
+ scan_task.scan_func();
+ return Status::OK();
+ }
+ Status submit_scan_task(SimplifiedScanTask scan_task, const std::string&)
override {
+ scan_task.scan_func();
+ return Status::OK();
+ }
+ void reset_thread_num(int, int, int) override {}
+ int get_queue_size() override { return 0; }
+ int get_active_threads() override { return 0; }
+ std::vector<int> thread_debug_info() override { return {}; }
+ Status schedule_scan_task(std::shared_ptr<ScannerContext>,
std::shared_ptr<ScanTask>,
+ std::unique_lock<std::mutex>&) override {
+ return Status::OK();
+ }
+};
+
+// submit_external_scan_tasks() signals completion from a Defer, so a worker
that leaves
+// without publishing its status would still wake the waiter and the caller
would report
+// success over a partially filled result block.
+class SubmitExternalScanTasksTest : public RowIdStorageReaderTest {
+protected:
+ static constexpr size_t kTaskCount = 3;
+
+ static Status run_tasks(const std::function<Status(size_t)>& run_task) {
+ InlineScanScheduler scheduler;
+ std::counting_semaphore<> semaphore {kTaskCount};
+ return RowIdStorageReader::submit_external_scan_tasks(
Review Comment:
[P1] Move the private helper call into the friended fixture.
`submit_external_scan_tasks` is private and `RowIdStorageReader` only friends
`RowIdStorageReaderTest`. This call is in
`SubmitExternalScanTasksTest::run_tasks`, a member of a derived fixture; C++
friendship is not inherited, so `rowid_fetcher_test.cpp` fails access checking.
Put a forwarding `run_tasks` member in the friended base fixture or explicitly
friend this fixture.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
