From 5a470ed1590a03d195e1f0856dbf92b2812ae852 Mon Sep 17 00:00:00 2001 From: Mryange Date: Fri, 26 Jun 2026 23:10:20 +0800 Subject: [PATCH] upd --- be/src/core/column/column.h | 30 +++++++++----- be/src/core/column/column_array.h | 12 ++---- be/src/core/column/column_const.h | 2 +- be/src/core/column/column_map.h | 14 ++----- be/src/core/column/column_nullable.h | 12 ++---- be/src/core/column/column_struct.cpp | 4 +- be/src/core/column/column_struct.h | 2 +- be/src/core/column/column_variant.cpp | 20 +++++---- be/src/core/column/column_variant.h | 2 +- be/test/core/column/column_array_test.cpp | 23 +++++++++++ be/test/core/column/column_nullable_test.cpp | 43 ++++++++++++++++++++ 11 files changed, 113 insertions(+), 51 deletions(-) diff --git a/be/src/core/column/column.h b/be/src/core/column/column.h index 6e5b69d907789b..38112a1f9fe8e1 100644 --- a/be/src/core/column/column.h +++ b/be/src/core/column/column.h @@ -563,11 +563,27 @@ class IColumn : public COW { /// If the column contains subcolumns (such as Array, Nullable, etc), do callback on them. /// Shallow: doesn't do recursive calls; don't do call for itself. - using MutableColumnCallback = std::function; using ColumnCallback = std::function; - virtual void for_each_subcolumn(MutableColumnCallback) {} virtual void for_each_subcolumn(ColumnCallback) const {} +protected: + virtual void mutate_subcolumns() {} + + static void mutate_subcolumn(WrappedPtr& subcolumn) { + static_cast(subcolumn) = + std::move(*static_cast(subcolumn)).mutate(); + } + + template + static void mutate_subcolumn(typename ColumnType::WrappedPtr& subcolumn) { + auto mutated = std::move(*static_cast(subcolumn)).mutate(); + auto typed_mutated = ColumnType::cast_to_column_mutptr( + assert_cast(mutated.get())); + mutated = nullptr; + static_cast(subcolumn) = std::move(typed_mutated); + } + +public: /// Columns have equal structure. /// If true - you can use "compare_at", "insert_from", etc. methods. virtual bool structure_equals(const IColumn&) const { @@ -581,10 +597,7 @@ class IColumn : public COW { // exclusive nodes are reused through the COW fast path. MutablePtr mutate() const&& { MutablePtr res = shallow_mutate(); - res->for_each_subcolumn([](WrappedPtr& subcolumn) { - static_cast(subcolumn) = - std::move(*static_cast(subcolumn)).mutate(); - }); + res->mutate_subcolumns(); return res; } @@ -595,10 +608,7 @@ class IColumn : public COW { static MutablePtr mutate(Ptr ptr) { MutablePtr res = ptr->shallow_mutate(); /// Now use_count is 2. ptr.reset(); /// Reset use_count to 1. - res->for_each_subcolumn([](WrappedPtr& subcolumn) { - static_cast(subcolumn) = - std::move(*static_cast(subcolumn)).mutate(); - }); + res->mutate_subcolumns(); return res; } diff --git a/be/src/core/column/column_array.h b/be/src/core/column/column_array.h index 0f2ef78a26c62e..645b1662d7b238 100644 --- a/be/src/core/column/column_array.h +++ b/be/src/core/column/column_array.h @@ -37,7 +37,6 @@ #include "core/field.h" #include "core/string_ref.h" #include "core/types.h" -#include "util/defer_op.h" class SipHash; @@ -216,14 +215,9 @@ class ColumnArray final : public COWHelper { return get_offsets()[i] - get_offsets()[i - 1]; } - void for_each_subcolumn(MutableColumnCallback callback) override { - IColumn::WrappedPtr offsets_column(std::move(static_cast(offsets))); - Defer defer([&] { - static_cast(offsets) = - cast_to_column(static_cast(offsets_column)); - }); - callback(offsets_column); - callback(data); + void mutate_subcolumns() override { + mutate_subcolumn(offsets); + mutate_subcolumn(data); } void for_each_subcolumn(ColumnCallback callback) const override { diff --git a/be/src/core/column/column_const.h b/be/src/core/column/column_const.h index fa647aca466799..834bfc2a08c899 100644 --- a/be/src/core/column/column_const.h +++ b/be/src/core/column/column_const.h @@ -261,7 +261,7 @@ class ColumnConst final : public COWHelper { } } - void for_each_subcolumn(MutableColumnCallback callback) override { callback(data); } + void mutate_subcolumns() override { mutate_subcolumn(data); } void for_each_subcolumn(ColumnCallback callback) const override { callback(*static_cast(data)); diff --git a/be/src/core/column/column_map.h b/be/src/core/column/column_map.h index d8f40a8fa98356..0781a2cb503569 100644 --- a/be/src/core/column/column_map.h +++ b/be/src/core/column/column_map.h @@ -43,7 +43,6 @@ #include "core/string_ref.h" #include "core/types.h" #include "exec/common/sip_hash.h" -#include "util/defer_op.h" class SipHash; @@ -74,15 +73,10 @@ class ColumnMap final : public COWHelper { std::string get_name() const override; - void for_each_subcolumn(MutableColumnCallback callback) override { - IColumn::WrappedPtr offsets(std::move(static_cast(offsets_column))); - Defer defer([&] { - static_cast(offsets_column) = - cast_to_column(static_cast(offsets)); - }); - callback(keys_column); - callback(values_column); - callback(offsets); + void mutate_subcolumns() override { + mutate_subcolumn(keys_column); + mutate_subcolumn(values_column); + mutate_subcolumn(offsets_column); } void for_each_subcolumn(ColumnCallback callback) const override { diff --git a/be/src/core/column/column_nullable.h b/be/src/core/column/column_nullable.h index 2161260bf9f1b6..27679d7bed38c7 100644 --- a/be/src/core/column/column_nullable.h +++ b/be/src/core/column/column_nullable.h @@ -32,7 +32,6 @@ #include "core/typeid_cast.h" #include "core/types.h" #include "storage/olap_common.h" -#include "util/defer_op.h" class SipHash; @@ -250,14 +249,9 @@ class ColumnNullable final : public COWHelper { return get_ptr(); } - void for_each_subcolumn(MutableColumnCallback callback) override { - callback(_nested_column); - - IColumn::WrappedPtr null_map(std::move(static_cast(_null_map))); - Defer defer([&] { - _null_map = cast_to_column(static_cast(null_map)); - }); - callback(null_map); + void mutate_subcolumns() override { + mutate_subcolumn(_nested_column); + mutate_subcolumn(_null_map); } void for_each_subcolumn(ColumnCallback callback) const override { diff --git a/be/src/core/column/column_struct.cpp b/be/src/core/column/column_struct.cpp index 9b0c5d2d27ad76..fb15785df8bb2d 100644 --- a/be/src/core/column/column_struct.cpp +++ b/be/src/core/column/column_struct.cpp @@ -379,9 +379,9 @@ bool ColumnStruct::has_enough_capacity(const IColumn& src) const { return true; } -void ColumnStruct::for_each_subcolumn(MutableColumnCallback callback) { +void ColumnStruct::mutate_subcolumns() { for (auto& column : columns) { - callback(column); + mutate_subcolumn(column); } } diff --git a/be/src/core/column/column_struct.h b/be/src/core/column/column_struct.h index 19b5259ee50576..83affe7296558f 100644 --- a/be/src/core/column/column_struct.h +++ b/be/src/core/column/column_struct.h @@ -155,7 +155,7 @@ class ColumnStruct final : public COWHelper { size_t byte_size() const override; size_t allocated_bytes() const override; bool has_enough_capacity(const IColumn& src) const override; - void for_each_subcolumn(MutableColumnCallback callback) override; + void mutate_subcolumns() override; void for_each_subcolumn(ColumnCallback callback) const override; bool structure_equals(const IColumn& rhs) const override; diff --git a/be/src/core/column/column_variant.cpp b/be/src/core/column/column_variant.cpp index d4f1cb1e4bd627..ec34c619cdc14b 100644 --- a/be/src/core/column/column_variant.cpp +++ b/be/src/core/column/column_variant.cpp @@ -68,7 +68,6 @@ #include "exprs/aggregate/aggregate_function.h" #include "exprs/json_functions.h" #include "storage/olap_common.h" -#include "util/defer_op.h" #include "util/json/path_in_data.h" #include "util/jsonb_document.h" #include "util/jsonb_document_cast.h" @@ -826,15 +825,14 @@ size_t ColumnVariant::allocated_bytes() const { return res; } -void ColumnVariant::for_each_subcolumn(MutableColumnCallback callback) { +void ColumnVariant::mutate_subcolumns() { for (auto& entry : subcolumns) { for (auto& part : entry->data.data) { - callback(part); + mutate_subcolumn(part); } } - callback(serialized_sparse_column); - callback(serialized_doc_value_column); - // callback may be filter, so the row count may be changed + mutate_subcolumn(serialized_sparse_column); + mutate_subcolumn(serialized_doc_value_column); num_rows = serialized_sparse_column->size(); ENABLE_CHECK_CONSISTENCY(this); } @@ -2385,7 +2383,7 @@ size_t ColumnVariant::filter(const Filter& filter) { for (auto& subcolumn : subcolumns) { subcolumn->data.num_rows = count; } - MutableColumnCallback callback = [&](IColumn::WrappedPtr& part) { + auto filter_part = [&](IColumn::WrappedPtr& part) { if (part->size() != count) { if (part->is_exclusive()) { const auto result_size = part->filter(filter); @@ -2401,7 +2399,13 @@ size_t ColumnVariant::filter(const Filter& filter) { } } }; - for_each_subcolumn(callback); + for (auto& entry : subcolumns) { + for (auto& part : entry->data.data) { + filter_part(part); + } + } + filter_part(serialized_sparse_column); + filter_part(serialized_doc_value_column); } num_rows = count; ENABLE_CHECK_CONSISTENCY(this); diff --git a/be/src/core/column/column_variant.h b/be/src/core/column/column_variant.h index cccf4d4031d303..a650555f49ec26 100644 --- a/be/src/core/column/column_variant.h +++ b/be/src/core/column/column_variant.h @@ -469,7 +469,7 @@ class ColumnVariant final : public COWHelper { bool has_enough_capacity(const IColumn& src) const override { return false; } - void for_each_subcolumn(MutableColumnCallback callback) override; + void mutate_subcolumns() override; void for_each_subcolumn(ColumnCallback callback) const override; // Do nothing, call try_insert instead diff --git a/be/test/core/column/column_array_test.cpp b/be/test/core/column/column_array_test.cpp index bef1cbdca3670c..6cfe107ac5ec1b 100644 --- a/be/test/core/column/column_array_test.cpp +++ b/be/test/core/column/column_array_test.cpp @@ -15,11 +15,14 @@ // specific language governing permissions and limitations // under the License. +#include "core/column/column_array.h" + #include #include #include #include "core/column/column.h" +#include "core/column/column_vector.h" #include "core/column/common_column_test.h" #include "core/types.h" #include "exprs/function/function_test_util.h" @@ -28,6 +31,26 @@ // for example column_ip should test these functions namespace doris { + +TEST(ColumnArrayCowTest, MutateKeepsExclusiveSubcolumns) { + auto data = ColumnInt64::create(); + data->insert_value(10); + + auto offsets = ColumnArray::ColumnOffsets::create(); + offsets->insert_value(1); + + ColumnPtr array = ColumnArray::create(std::move(data), std::move(offsets)); + const auto& array_ref = assert_cast(*array); + const auto* data_raw = array_ref.get_data_ptr().get(); + const auto* offsets_raw = array_ref.get_offsets_ptr().get(); + + auto mutated = IColumn::mutate(std::move(array)); + const auto& mutated_array = assert_cast(*mutated); + + EXPECT_EQ(mutated_array.get_data_ptr().get(), data_raw); + EXPECT_EQ(mutated_array.get_offsets_ptr().get(), offsets_raw); +} + static std::string test_result_dir; class ColumnArrayTest : public CommonColumnTest { protected: diff --git a/be/test/core/column/column_nullable_test.cpp b/be/test/core/column/column_nullable_test.cpp index 6e6a7d8b4f960b..8d8cfc35818785 100644 --- a/be/test/core/column/column_nullable_test.cpp +++ b/be/test/core/column/column_nullable_test.cpp @@ -130,6 +130,49 @@ TEST(ColumnNullableTest, SharedCreatePreservesImmutableSubcolumns) { EXPECT_EQ(null_map_alias->size(), 1); } +TEST(ColumnNullableTest, MutateDetachesTypedNullMap) { + auto nested_mut = ColumnInt64::create(); + nested_mut->insert_value(10); + ColumnPtr nested = std::move(nested_mut); + ColumnPtr nested_alias = nested; + + auto null_map_mut = ColumnUInt8::create(); + null_map_mut->insert_value(0); + ColumnPtr null_map = std::move(null_map_mut); + ColumnPtr null_map_alias = null_map; + + ColumnPtr nullable = ColumnNullable::create(nested, null_map); + auto mutated = IColumn::mutate(nullable); + auto& mutated_nullable = assert_cast(*mutated); + + EXPECT_NE(mutated_nullable.get_nested_column_ptr().get(), nested_alias.get()); + EXPECT_NE(mutated_nullable.get_null_map_column_ptr().get(), null_map_alias.get()); + EXPECT_EQ(mutated_nullable.get_null_map_data()[0], 0); + + mutated_nullable.get_null_map_data()[0] = 1; + const auto& original_null_map = assert_cast(*null_map_alias); + EXPECT_EQ(original_null_map.get_data()[0], 0); +} + +TEST(ColumnNullableTest, MutateKeepsExclusiveSubcolumns) { + auto nested_mut = ColumnInt64::create(); + nested_mut->insert_value(10); + + auto null_map_mut = ColumnUInt8::create(); + null_map_mut->insert_value(0); + + ColumnPtr nullable = ColumnNullable::create(std::move(nested_mut), std::move(null_map_mut)); + const auto& nullable_ref = assert_cast(*nullable); + const auto* nested_raw = nullable_ref.get_nested_column_ptr().get(); + const auto* null_map_raw = nullable_ref.get_null_map_column_ptr().get(); + + auto mutated = IColumn::mutate(std::move(nullable)); + const auto& mutated_nullable = assert_cast(*mutated); + + EXPECT_EQ(mutated_nullable.get_nested_column_ptr().get(), nested_raw); + EXPECT_EQ(mutated_nullable.get_null_map_column_ptr().get(), null_map_raw); +} + TEST(ColumnNullableTest, append_data_by_selector) { auto srt_column = ColumnHelper::create_nullable_column( {1, 2, 3, 4, 5, 6, 7, 8, 9, 10},