This is an automated email from the ASF dual-hosted git repository. yiguolei pushed a commit to branch branch-2.1 in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-2.1 by this push: new 521e653631d [Chore](check) add error check for agg state with some function use special serializ… (#46552) 521e653631d is described below commit 521e653631d94975c0f44fd6f82e30206cc6497e Author: Pxl <x...@selectdb.com> AuthorDate: Thu Jan 9 11:10:44 2025 +0800 [Chore](check) add error check for agg state with some function use special serializ… (#46552) --- be/src/agent/be_exec_version_manager.h | 1 - .../aggregate_function_bitmap.h | 113 +++++++-------------- be/src/vec/data_types/data_type_agg_state.h | 6 ++ .../agg_with_roll_up/aggregate_with_roll_up.groovy | 30 ++++++ 4 files changed, 74 insertions(+), 76 deletions(-) diff --git a/be/src/agent/be_exec_version_manager.h b/be/src/agent/be_exec_version_manager.h index 709e101178a..af43a2d5c8f 100644 --- a/be/src/agent/be_exec_version_manager.h +++ b/be/src/agent/be_exec_version_manager.h @@ -76,7 +76,6 @@ constexpr inline int BeExecVersionManager::max_be_exec_version = 6; constexpr inline int BeExecVersionManager::min_be_exec_version = 0; /// functional -constexpr inline int BITMAP_SERDE = 3; constexpr inline int USE_NEW_SERDE = 4; // release on DORIS version 2.1 constexpr inline int OLD_WAL_SERDE = 3; // use to solve compatibility issues, see pr #32299 constexpr inline int VARIANT_SERDE = 6; // change variant serde to fix PR #38413 diff --git a/be/src/vec/aggregate_functions/aggregate_function_bitmap.h b/be/src/vec/aggregate_functions/aggregate_function_bitmap.h index dd7af71de06..b1357a7a7dc 100644 --- a/be/src/vec/aggregate_functions/aggregate_function_bitmap.h +++ b/be/src/vec/aggregate_functions/aggregate_function_bitmap.h @@ -160,48 +160,36 @@ public: void streaming_agg_serialize_to_column(const IColumn** columns, MutableColumnPtr& dst, const size_t num_rows, Arena* arena) const override { - if (version >= BITMAP_SERDE) { - auto& col = assert_cast<ColumnBitmap&>(*dst); - char place[sizeof(Data)]; - col.resize(num_rows); - auto* data = col.get_data().data(); - for (size_t i = 0; i != num_rows; ++i) { - assert_cast<const Derived*>(this)->create(place); - DEFER({ assert_cast<const Derived*>(this)->destroy(place); }); - assert_cast<const Derived*>(this)->add(place, columns, i, arena); - data[i] = std::move(this->data(place).value); - } - } else { - BaseHelper::streaming_agg_serialize_to_column(columns, dst, num_rows, arena); + auto& col = assert_cast<ColumnBitmap&>(*dst); + char place[sizeof(Data)]; + col.resize(num_rows); + auto* data = col.get_data().data(); + for (size_t i = 0; i != num_rows; ++i) { + assert_cast<const Derived*>(this)->create(place); + DEFER({ assert_cast<const Derived*>(this)->destroy(place); }); + assert_cast<const Derived*>(this)->add(place, columns, i, arena); + data[i] = std::move(this->data(place).value); } } void serialize_to_column(const std::vector<AggregateDataPtr>& places, size_t offset, MutableColumnPtr& dst, const size_t num_rows) const override { - if (version >= BITMAP_SERDE) { - auto& col = assert_cast<ColumnBitmap&>(*dst); - col.resize(num_rows); - auto* data = col.get_data().data(); - for (size_t i = 0; i != num_rows; ++i) { - data[i] = std::move(this->data(places[i] + offset).value); - } - } else { - BaseHelper::serialize_to_column(places, offset, dst, num_rows); + auto& col = assert_cast<ColumnBitmap&>(*dst); + col.resize(num_rows); + auto* data = col.get_data().data(); + for (size_t i = 0; i != num_rows; ++i) { + data[i] = std::move(this->data(places[i] + offset).value); } } void deserialize_and_merge_from_column(AggregateDataPtr __restrict place, const IColumn& column, Arena* arena) const override { - if (version >= BITMAP_SERDE) { - auto& col = assert_cast<const ColumnBitmap&>(column); - const size_t num_rows = column.size(); - auto* data = col.get_data().data(); + auto& col = assert_cast<const ColumnBitmap&>(column); + const size_t num_rows = column.size(); + auto* data = col.get_data().data(); - for (size_t i = 0; i != num_rows; ++i) { - this->data(place).merge(data[i]); - } - } else { - BaseHelper::deserialize_and_merge_from_column(place, column, arena); + for (size_t i = 0; i != num_rows; ++i) { + this->data(place).merge(data[i]); } } @@ -210,74 +198,49 @@ public: Arena* arena) const override { DCHECK(end <= column.size() && begin <= end) << ", begin:" << begin << ", end:" << end << ", column.size():" << column.size(); - if (version >= BITMAP_SERDE) { - auto& col = assert_cast<const ColumnBitmap&>(column); - auto* data = col.get_data().data(); - for (size_t i = begin; i <= end; ++i) { - this->data(place).merge(data[i]); - } - } else { - BaseHelper::deserialize_and_merge_from_column_range(place, column, begin, end, arena); + auto& col = assert_cast<const ColumnBitmap&>(column); + auto* data = col.get_data().data(); + for (size_t i = begin; i <= end; ++i) { + this->data(place).merge(data[i]); } } void deserialize_and_merge_vec(const AggregateDataPtr* places, size_t offset, AggregateDataPtr rhs, const IColumn* column, Arena* arena, const size_t num_rows) const override { - if (version >= BITMAP_SERDE) { - const auto& col = assert_cast<const ColumnBitmap&>(*column); - const auto* data = col.get_data().data(); - for (size_t i = 0; i != num_rows; ++i) { - this->data(places[i] + offset).merge(data[i]); - } - } else { - BaseHelper::deserialize_and_merge_vec(places, offset, rhs, column, arena, num_rows); + const auto& col = assert_cast<const ColumnBitmap&>(*column); + const auto* data = col.get_data().data(); + for (size_t i = 0; i != num_rows; ++i) { + this->data(places[i] + offset).merge(data[i]); } } void deserialize_and_merge_vec_selected(const AggregateDataPtr* places, size_t offset, AggregateDataPtr rhs, const IColumn* column, Arena* arena, const size_t num_rows) const override { - if (version >= BITMAP_SERDE) { - const auto& col = assert_cast<const ColumnBitmap&>(*column); - const auto* data = col.get_data().data(); - for (size_t i = 0; i != num_rows; ++i) { - if (places[i]) { - this->data(places[i] + offset).merge(data[i]); - } + const auto& col = assert_cast<const ColumnBitmap&>(*column); + const auto* data = col.get_data().data(); + for (size_t i = 0; i != num_rows; ++i) { + if (places[i]) { + this->data(places[i] + offset).merge(data[i]); } - } else { - BaseHelper::deserialize_and_merge_vec_selected(places, offset, rhs, column, arena, - num_rows); } } void serialize_without_key_to_column(ConstAggregateDataPtr __restrict place, IColumn& to) const override { - if (version >= BITMAP_SERDE) { - auto& col = assert_cast<ColumnBitmap&>(to); - size_t old_size = col.size(); - col.resize(old_size + 1); - col.get_data()[old_size] = std::move(this->data(place).value); - } else { - BaseHelper::serialize_without_key_to_column(place, to); - } + auto& col = assert_cast<ColumnBitmap&>(to); + size_t old_size = col.size(); + col.resize(old_size + 1); + col.get_data()[old_size] = std::move(this->data(place).value); } [[nodiscard]] MutableColumnPtr create_serialize_column() const override { - if (version >= BITMAP_SERDE) { - return ColumnBitmap::create(); - } else { - return ColumnString::create(); - } + return ColumnBitmap::create(); } [[nodiscard]] DataTypePtr get_serialized_type() const override { - if (version >= BITMAP_SERDE) { - return std::make_shared<DataTypeBitMap>(); - } else { - return IAggregateFunction::get_serialized_type(); - } + return std::make_shared<DataTypeBitMap>(); } protected: diff --git a/be/src/vec/data_types/data_type_agg_state.h b/be/src/vec/data_types/data_type_agg_state.h index ff6f1975e58..91249202c02 100644 --- a/be/src/vec/data_types/data_type_agg_state.h +++ b/be/src/vec/data_types/data_type_agg_state.h @@ -46,6 +46,12 @@ public: "DataTypeAggState function get failed, type={}", do_get_name()); } _agg_serialized_type = _agg_function->get_serialized_type(); + if (_agg_serialized_type->get_type_as_type_descriptor().type != TYPE_STRING && + _agg_serialized_type->get_type_as_type_descriptor().type != INVALID_TYPE) { + throw Exception(ErrorCode::INTERNAL_ERROR, + "meet invalid serialized_type of agg state nested function {}", + function_name); + } } const char* get_family_name() const override { return "AggState"; } diff --git a/regression-test/suites/nereids_rules_p0/mv/agg_with_roll_up/aggregate_with_roll_up.groovy b/regression-test/suites/nereids_rules_p0/mv/agg_with_roll_up/aggregate_with_roll_up.groovy index 5b8aa6e00d2..b6c64d3376e 100644 --- a/regression-test/suites/nereids_rules_p0/mv/agg_with_roll_up/aggregate_with_roll_up.groovy +++ b/regression-test/suites/nereids_rules_p0/mv/agg_with_roll_up/aggregate_with_roll_up.groovy @@ -1070,6 +1070,36 @@ suite("aggregate_with_roll_up") { order_qt_query26_0_before "${query26_0}" async_mv_rewrite_fail(db, mv26_0, query26_0, "mv26_0") order_qt_query26_0_after "${query26_0}" + + sql """ + CREATE MATERIALIZED VIEW `table_mv` AS select o_orderdate, o_shippriority, o_comment, + sum(o_totalprice) as sum_total, + max(o_totalprice) as max_total, + min(o_totalprice) as min_total, + count(*) as count_all, + bitmap_union_count(to_bitmap(case when o_shippriority > 1 and o_orderkey IN (1, 3) then o_custkey else null end)) cnt_1, + bitmap_union_count(to_bitmap(case when o_shippriority > 2 and o_orderkey IN (2) then o_custkey else null end)) as cnt_2 + from orders + group by + o_orderdate, + o_shippriority, + o_comment; + """ + Thread.sleep(1000) + + mv_rewrite_fail("""select o_orderdate, o_shippriority, o_comment, + sum(o_totalprice) as sum_total, + max(o_totalprice) as max_total, + min(o_totalprice) as min_total, + count(*) as count_all, + bitmap_union_count(to_bitmap(case when o_shippriority > 1 and o_orderkey IN (1, 3) then o_custkey else null end)) cnt_1, + bitmap_union_count(to_bitmap(case when o_shippriority > 2 and o_orderkey IN (2) then o_custkey else null end)) as cnt_2 + from orders + group by + o_orderdate, + o_shippriority, + o_comment;""", "table_mv") + sql """ DROP MATERIALIZED VIEW IF EXISTS mv26_0""" --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org