From 5312aac213e6bf1afe3c2efe35dc319b9a236c6a Mon Sep 17 00:00:00 2001 From: robot-clickhouse Date: Mon, 28 Oct 2024 10:10:29 +0000 Subject: [PATCH] Backport #70820 to 24.8: Fix a crash and a leak in AggregateFunctionGroupArraySorted --- .../AggregateFunctionGroupArraySorted.cpp | 45 +++++++++++-------- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/src/AggregateFunctions/AggregateFunctionGroupArraySorted.cpp b/src/AggregateFunctions/AggregateFunctionGroupArraySorted.cpp index 27043ed6aa6..813a777ea7a 100644 --- a/src/AggregateFunctions/AggregateFunctionGroupArraySorted.cpp +++ b/src/AggregateFunctions/AggregateFunctionGroupArraySorted.cpp @@ -59,13 +59,13 @@ constexpr size_t group_array_sorted_sort_strategy_max_elements_threshold = 10000 template struct GroupArraySortedData { + static constexpr bool is_value_generic_field = std::is_same_v; + using Allocator = MixedAlignedArenaAllocator; - using Array = PODArray; + using Array = typename std::conditional_t, PODArray>; static constexpr size_t partial_sort_max_elements_factor = 2; - static constexpr bool is_value_generic_field = std::is_same_v; - Array values; static bool compare(const T & lhs, const T & rhs) @@ -144,7 +144,7 @@ struct GroupArraySortedData } if (values.size() > max_elements) - values.resize(max_elements, arena); + resize(max_elements, arena); } ALWAYS_INLINE void partialSortAndLimitIfNeeded(size_t max_elements, Arena * arena) @@ -153,7 +153,23 @@ struct GroupArraySortedData return; ::nth_element(values.begin(), values.begin() + max_elements, values.end(), Comparator()); - values.resize(max_elements, arena); + resize(max_elements, arena); + } + + ALWAYS_INLINE void resize(size_t n, Arena * arena) + { + if constexpr (is_value_generic_field) + values.resize(n); + else + values.resize(n, arena); + } + + ALWAYS_INLINE void push_back(T && element, Arena * arena) + { + if constexpr (is_value_generic_field) + values.push_back(element); + else + values.push_back(element, arena); } ALWAYS_INLINE void addElement(T && element, size_t max_elements, Arena * arena) @@ -171,12 +187,12 @@ struct GroupArraySortedData return; } - values.push_back(std::move(element), arena); + push_back(std::move(element), arena); std::push_heap(values.begin(), values.end(), Comparator()); } else { - values.push_back(std::move(element), arena); + push_back(std::move(element), arena); partialSortAndLimitIfNeeded(max_elements, arena); } } @@ -210,14 +226,6 @@ struct GroupArraySortedData result_array_data[result_array_data_insert_begin + i] = values[i]; } } - - ~GroupArraySortedData() - { - for (auto & value : values) - { - value.~T(); - } - } }; template @@ -313,14 +321,12 @@ public: throw Exception(ErrorCodes::TOO_LARGE_ARRAY_SIZE, "Too large array size, it should not exceed {}", max_elements); auto & values = this->data(place).values; - values.resize_exact(size, arena); - if constexpr (std::is_same_v) + if constexpr (Data::is_value_generic_field) { + values.resize(size); for (Field & element : values) { - /// We must initialize the Field type since some internal functions (like operator=) use them - new (&element) Field; bool has_value = false; readBinary(has_value, buf); if (has_value) @@ -329,6 +335,7 @@ public: } else { + values.resize_exact(size, arena); if constexpr (std::endian::native == std::endian::little) { buf.readStrict(reinterpret_cast(values.data()), size * sizeof(values[0]));