From 77cacdb5488886daa9303a31aa5ae919f92eb9fa Mon Sep 17 00:00:00 2001 From: root Date: Mon, 1 Jun 2026 10:22:38 +0800 Subject: [PATCH] [fix](storage) Fix linker error for SegmentWriter::_is_mow() with -O1/-O3 --- be/src/storage/segment/segment_writer.cpp | 8 - be/src/storage/segment/segment_writer.h | 8 +- .../segment/vertical_segment_writer.cpp | 8 - .../storage/segment/vertical_segment_writer.h | 8 +- .../segment/segment_writer_mow_check_test.cpp | 200 ++++++++++++++++++ 5 files changed, 212 insertions(+), 20 deletions(-) create mode 100644 be/test/storage/segment/segment_writer_mow_check_test.cpp diff --git a/be/src/storage/segment/segment_writer.cpp b/be/src/storage/segment/segment_writer.cpp index 00db2362b63fff..f5f63d923f7750 100644 --- a/be/src/storage/segment/segment_writer.cpp +++ b/be/src/storage/segment/segment_writer.cpp @@ -1244,13 +1244,5 @@ Status SegmentWriter::_generate_short_key_index(std::vectorkeys_type() == UNIQUE_KEYS && _opts.enable_unique_key_merge_on_write; -} - -bool SegmentWriter::_is_mow_with_cluster_key() { - return _is_mow() && !_tablet_schema->cluster_key_uids().empty(); -} - } // namespace segment_v2 } // namespace doris diff --git a/be/src/storage/segment/segment_writer.h b/be/src/storage/segment/segment_writer.h index c98a6cead20b8a..e6bba442bd4a10 100644 --- a/be/src/storage/segment/segment_writer.h +++ b/be/src/storage/segment/segment_writer.h @@ -189,8 +189,12 @@ class SegmentWriter { IOlapColumnDataAccessor* seq_column, size_t num_rows, bool need_sort); Status _generate_short_key_index(std::vector& key_columns, size_t num_rows, const std::vector& short_key_pos); - bool _is_mow(); - bool _is_mow_with_cluster_key(); + bool _is_mow() { + return _tablet_schema->keys_type() == UNIQUE_KEYS && _opts.enable_unique_key_merge_on_write; + } + bool _is_mow_with_cluster_key() { + return _is_mow() && !_tablet_schema->cluster_key_uids().empty(); + } protected: // Build key index for derived writers that override append_block. diff --git a/be/src/storage/segment/vertical_segment_writer.cpp b/be/src/storage/segment/vertical_segment_writer.cpp index fed06831fbd053..6cf4cf1a1c6ed7 100644 --- a/be/src/storage/segment/vertical_segment_writer.cpp +++ b/be/src/storage/segment/vertical_segment_writer.cpp @@ -1516,12 +1516,4 @@ void VerticalSegmentWriter::_set_max_key(const Slice& key) { _max_key.append(key.get_data(), key.get_size()); } -inline bool VerticalSegmentWriter::_is_mow() { - return _tablet_schema->keys_type() == UNIQUE_KEYS && _opts.enable_unique_key_merge_on_write; -} - -inline bool VerticalSegmentWriter::_is_mow_with_cluster_key() { - return _is_mow() && !_tablet_schema->cluster_key_uids().empty(); -} - } // namespace doris::segment_v2 diff --git a/be/src/storage/segment/vertical_segment_writer.h b/be/src/storage/segment/vertical_segment_writer.h index e6ee4933dd6d19..1a0645efa92daf 100644 --- a/be/src/storage/segment/vertical_segment_writer.h +++ b/be/src/storage/segment/vertical_segment_writer.h @@ -199,8 +199,12 @@ class VerticalSegmentWriter { Status _check_column_writer_disk_capacity(size_t cid); Status _finalize_column_writer_and_update_meta(size_t cid); - bool _is_mow(); - bool _is_mow_with_cluster_key(); + bool _is_mow() { + return _tablet_schema->keys_type() == UNIQUE_KEYS && _opts.enable_unique_key_merge_on_write; + } + bool _is_mow_with_cluster_key() { + return _is_mow() && !_tablet_schema->cluster_key_uids().empty(); + } private: friend class ::doris::BlockAggregator; diff --git a/be/test/storage/segment/segment_writer_mow_check_test.cpp b/be/test/storage/segment/segment_writer_mow_check_test.cpp new file mode 100644 index 00000000000000..e8be43a7fa4533 --- /dev/null +++ b/be/test/storage/segment/segment_writer_mow_check_test.cpp @@ -0,0 +1,200 @@ +// 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. + +#include + +#include +#include + +#include "io/fs/local_file_system.h" +#include "storage/olap_common.h" +#include "storage/rowset/rowset_id_generator.h" +#include "storage/segment/segment_writer.h" +#include "storage/segment/vertical_segment_writer.h" +#include "storage/tablet/tablet_schema.h" + +namespace doris::segment_v2 { + +// Subclass in a different translation unit that calls _is_mow() and _is_mow_with_cluster_key(). +// This test verifies that these inline private methods are visible across translation units. +// Previously they were defined with `inline` in .cpp files, which caused linker errors +// when compiled with -O1 or higher (TSAN/RELEASE), because the compiler inlined them +// and did not export the symbols. +class TestSegmentWriterMowCheck : public SegmentWriter { +public: + using SegmentWriter::SegmentWriter; + + bool check_is_mow() { return _is_mow(); } + bool check_is_mow_with_cluster_key() { return _is_mow_with_cluster_key(); } +}; + +class TestVerticalSegmentWriterMowCheck : public VerticalSegmentWriter { +public: + using VerticalSegmentWriter::VerticalSegmentWriter; + + bool check_is_mow() { return _is_mow(); } + bool check_is_mow_with_cluster_key() { return _is_mow_with_cluster_key(); } +}; + +static const std::string kSegmentDir = "./ut_dir/segment_writer_mow_check_test"; + +TabletColumnPtr create_int_key(int32_t id) { + auto column = std::make_shared(); + column->_unique_id = id; + column->_col_name = std::to_string(id); + column->_type = FieldType::OLAP_FIELD_TYPE_INT; + column->_is_key = true; + column->_is_nullable = false; + column->_length = 4; + column->_index_length = 4; + return column; +} + +TabletColumnPtr create_int_value(int32_t id) { + auto column = std::make_shared(); + column->_unique_id = id; + column->_col_name = std::to_string(id); + column->_type = FieldType::OLAP_FIELD_TYPE_INT; + column->_is_key = false; + column->_is_nullable = true; + column->_length = 4; + column->_index_length = 4; + return column; +} + +TabletSchemaSPtr create_unique_key_schema() { + TabletSchemaSPtr schema = std::make_shared(); + schema->append_column(*create_int_key(0)); + schema->append_column(*create_int_value(1)); + schema->_keys_type = UNIQUE_KEYS; + return schema; +} + +TabletSchemaSPtr create_dup_key_schema() { + TabletSchemaSPtr schema = std::make_shared(); + schema->append_column(*create_int_key(0)); + schema->append_column(*create_int_value(1)); + schema->_keys_type = DUP_KEYS; + return schema; +} + +TabletSchemaSPtr create_unique_key_schema_with_cluster_key() { + auto schema = create_unique_key_schema(); + schema->_cluster_key_uids = {1}; + return schema; +} + +class SegmentWriterMowCheckTest : public testing::Test { +public: + void SetUp() override { + auto fs = io::global_local_filesystem(); + auto st = fs->delete_directory(kSegmentDir); + ASSERT_TRUE(st.ok() || st.is()) << st; + st = fs->create_directory(kSegmentDir); + ASSERT_TRUE(st.ok()) << st; + } + + void TearDown() override { + EXPECT_TRUE(io::global_local_filesystem()->delete_directory(kSegmentDir).ok()); + } + + io::FileWriterPtr create_file_writer(size_t segment_id) { + RowsetId rowset_id; + rowset_id.init(1); + std::string filename = fmt::format("{}_{}.dat", rowset_id.to_string(), segment_id); + std::string path = fmt::format("{}/{}", kSegmentDir, filename); + io::FileWriterPtr file_writer; + auto st = io::global_local_filesystem()->create_file(path, &file_writer); + EXPECT_TRUE(st.ok()) << st; + return file_writer; + } +}; + +TEST_F(SegmentWriterMowCheckTest, segment_writer_is_mow_false_for_dup_key) { + auto schema = create_dup_key_schema(); + SegmentWriterOptions opts; + opts.enable_unique_key_merge_on_write = true; + auto file_writer = create_file_writer(0); + TestSegmentWriterMowCheck writer(file_writer.get(), 0, schema, nullptr, nullptr, opts, nullptr); + EXPECT_FALSE(writer.check_is_mow()); + EXPECT_FALSE(writer.check_is_mow_with_cluster_key()); +} + +TEST_F(SegmentWriterMowCheckTest, segment_writer_is_mow_true_for_unique_mow) { + auto schema = create_unique_key_schema(); + SegmentWriterOptions opts; + opts.enable_unique_key_merge_on_write = true; + auto file_writer = create_file_writer(1); + TestSegmentWriterMowCheck writer(file_writer.get(), 1, schema, nullptr, nullptr, opts, nullptr); + EXPECT_TRUE(writer.check_is_mow()); + EXPECT_FALSE(writer.check_is_mow_with_cluster_key()); +} + +TEST_F(SegmentWriterMowCheckTest, segment_writer_is_mow_false_when_mow_disabled) { + auto schema = create_unique_key_schema(); + SegmentWriterOptions opts; + opts.enable_unique_key_merge_on_write = false; + auto file_writer = create_file_writer(2); + TestSegmentWriterMowCheck writer(file_writer.get(), 2, schema, nullptr, nullptr, opts, nullptr); + EXPECT_FALSE(writer.check_is_mow()); + EXPECT_FALSE(writer.check_is_mow_with_cluster_key()); +} + +TEST_F(SegmentWriterMowCheckTest, segment_writer_is_mow_with_cluster_key) { + auto schema = create_unique_key_schema_with_cluster_key(); + SegmentWriterOptions opts; + opts.enable_unique_key_merge_on_write = true; + auto file_writer = create_file_writer(3); + TestSegmentWriterMowCheck writer(file_writer.get(), 3, schema, nullptr, nullptr, opts, nullptr); + EXPECT_TRUE(writer.check_is_mow()); + EXPECT_TRUE(writer.check_is_mow_with_cluster_key()); +} + +TEST_F(SegmentWriterMowCheckTest, vertical_segment_writer_is_mow_false_for_dup_key) { + auto schema = create_dup_key_schema(); + VerticalSegmentWriterOptions opts; + opts.enable_unique_key_merge_on_write = true; + auto file_writer = create_file_writer(4); + TestVerticalSegmentWriterMowCheck writer(file_writer.get(), 4, schema, nullptr, nullptr, opts, + nullptr); + EXPECT_FALSE(writer.check_is_mow()); + EXPECT_FALSE(writer.check_is_mow_with_cluster_key()); +} + +TEST_F(SegmentWriterMowCheckTest, vertical_segment_writer_is_mow_true_for_unique_mow) { + auto schema = create_unique_key_schema(); + VerticalSegmentWriterOptions opts; + opts.enable_unique_key_merge_on_write = true; + auto file_writer = create_file_writer(5); + TestVerticalSegmentWriterMowCheck writer(file_writer.get(), 5, schema, nullptr, nullptr, opts, + nullptr); + EXPECT_TRUE(writer.check_is_mow()); + EXPECT_FALSE(writer.check_is_mow_with_cluster_key()); +} + +TEST_F(SegmentWriterMowCheckTest, vertical_segment_writer_is_mow_with_cluster_key) { + auto schema = create_unique_key_schema_with_cluster_key(); + VerticalSegmentWriterOptions opts; + opts.enable_unique_key_merge_on_write = true; + auto file_writer = create_file_writer(6); + TestVerticalSegmentWriterMowCheck writer(file_writer.get(), 6, schema, nullptr, nullptr, opts, + nullptr); + EXPECT_TRUE(writer.check_is_mow()); + EXPECT_TRUE(writer.check_is_mow_with_cluster_key()); +} + +} // namespace doris::segment_v2