From 9481d21b44191f817e73c3c000fb59b488827783 Mon Sep 17 00:00:00 2001 From: zhangstar333 <87313068+zhangstar333@users.noreply.github.com> Date: Fri, 6 Sep 2024 10:59:23 +0800 Subject: [PATCH 1/2] [Bug](compatibility) fix stddev_samp function coredump when upgrade (#40438) ## Proposed changes ``` if (IAggregateFunction::version < AGG_FUNCTION_NULLABLE) { return make_nullable(Data::get_return_type()); } else { return Data::get_return_type(); } ``` before check two version , and then get return type, but now in branch-21, the IAggregateFunction::version have update to 5, and also AGG_FUNCTION_NEW=5 so the check will not get right return type. --- .../aggregate_function_stddev.h | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/be/src/vec/aggregate_functions/aggregate_function_stddev.h b/be/src/vec/aggregate_functions/aggregate_function_stddev.h index 6af310b0ae8fad..5a8c896e1c3247 100644 --- a/be/src/vec/aggregate_functions/aggregate_function_stddev.h +++ b/be/src/vec/aggregate_functions/aggregate_function_stddev.h @@ -126,8 +126,6 @@ struct BaseData { count += 1; } - static DataTypePtr get_return_type() { return std::make_shared>(); } - double mean; double m2; int64_t count; @@ -145,6 +143,8 @@ struct PopData : Data { col.get_data().push_back(this->get_pop_result()); } } + + static DataTypePtr get_return_type() { return std::make_shared>(); } }; // For this series of functions, the Decimal type is not supported @@ -189,6 +189,10 @@ struct SampData_OLDER : Data { nullable_column.get_null_map_data().push_back(0); } } + + static DataTypePtr get_return_type() { + return make_nullable(std::make_shared>()); + } }; template @@ -207,6 +211,8 @@ struct SampData : Data { } } } + + static DataTypePtr get_return_type() { return std::make_shared>(); } }; template @@ -221,17 +227,7 @@ class AggregateFunctionSampVariance String get_name() const override { return Data::name(); } - DataTypePtr get_return_type() const override { - if constexpr (is_pop) { - return Data::get_return_type(); - } else { - if (IAggregateFunction::version < AGG_FUNCTION_NULLABLE) { - return make_nullable(Data::get_return_type()); - } else { - return Data::get_return_type(); - } - } - } + DataTypePtr get_return_type() const override { return Data::get_return_type(); } void add(AggregateDataPtr __restrict place, const IColumn** columns, ssize_t row_num, Arena*) const override { From f27a0f66174e15645dcac5d8770933ac78c6638b Mon Sep 17 00:00:00 2001 From: zhangstar333 <87313068+zhangstar333@users.noreply.github.com> Date: Sun, 8 Sep 2024 18:18:14 +0800 Subject: [PATCH 2/2] [Bug](compatibility) fix agg functions coredump when upgrade (#40472) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Proposed changes before use check (IAggregateFunction::version < AGG_FUNCTION_NULLABLE) and then choose the return type and insert method. but the version maybe update in the same branch-version，and get wrong type so put those function to all of object class alone. --- .../aggregate_function_covar.h | 19 +-- .../aggregate_function_percentile.h | 151 ++++++++++++++---- 2 files changed, 126 insertions(+), 44 deletions(-) diff --git a/be/src/vec/aggregate_functions/aggregate_function_covar.h b/be/src/vec/aggregate_functions/aggregate_function_covar.h index 179e723285e900..9b4b1b70c1fa7f 100644 --- a/be/src/vec/aggregate_functions/aggregate_function_covar.h +++ b/be/src/vec/aggregate_functions/aggregate_function_covar.h @@ -120,8 +120,6 @@ struct BaseData { count += 1; } - static DataTypePtr get_return_type() { return std::make_shared>(); } - double sum_x; double sum_y; double sum_xy; @@ -134,6 +132,7 @@ struct PopData : Data { auto& col = assert_cast(to); col.get_data().push_back(this->get_pop_result()); } + static DataTypePtr get_return_type() { return std::make_shared>(); } }; template @@ -148,6 +147,9 @@ struct SampData_OLDER : Data { nullable_column.get_null_map_data().push_back(0); } } + static DataTypePtr get_return_type() { + return make_nullable(std::make_shared>()); + } }; template @@ -160,6 +162,7 @@ struct SampData : Data { col.get_data().push_back(this->get_samp_result()); } } + static DataTypePtr get_return_type() { return std::make_shared>(); } }; template @@ -184,17 +187,7 @@ class AggregateFunctionSampCovariance String get_name() const override { return Data::name(); } - DataTypePtr get_return_type() const override { - if constexpr (is_pop) { - return Data::get_return_type(); - } else { - if (IAggregateFunction::version < AGG_FUNCTION_NULLABLE) { - return make_nullable(Data::get_return_type()); - } else { - return Data::get_return_type(); - } - } - } + DataTypePtr get_return_type() const override { return Data::get_return_type(); } void add(AggregateDataPtr __restrict place, const IColumn** columns, ssize_t row_num, Arena*) const override { diff --git a/be/src/vec/aggregate_functions/aggregate_function_percentile.h b/be/src/vec/aggregate_functions/aggregate_function_percentile.h index 5217ba5aeb5838..1c8a12340d7096 100644 --- a/be/src/vec/aggregate_functions/aggregate_function_percentile.h +++ b/be/src/vec/aggregate_functions/aggregate_function_percentile.h @@ -162,13 +162,6 @@ class AggregateFunctionPercentileApprox String get_name() const override { return "percentile_approx"; } - DataTypePtr get_return_type() const override { - if (IAggregateFunction::version < AGG_FUNCTION_NULLABLE) { - return make_nullable(std::make_shared()); - } - return std::make_shared(); - } - void reset(AggregateDataPtr __restrict place) const override { AggregateFunctionPercentileApprox::data(place).reset(); } @@ -187,30 +180,6 @@ class AggregateFunctionPercentileApprox Arena*) const override { AggregateFunctionPercentileApprox::data(place).read(buf); } - - void insert_result_into(ConstAggregateDataPtr __restrict place, IColumn& to) const override { - if (IAggregateFunction::version < AGG_FUNCTION_NULLABLE) { - ColumnNullable& nullable_column = assert_cast(to); - double result = AggregateFunctionPercentileApprox::data(place).get(); - - if (std::isnan(result)) { - nullable_column.insert_default(); - } else { - auto& col = assert_cast(nullable_column.get_nested_column()); - col.get_data().push_back(result); - nullable_column.get_null_map_data().push_back(0); - } - } else { - auto& col = assert_cast(to); - double result = AggregateFunctionPercentileApprox::data(place).get(); - - if (std::isnan(result)) { - col.insert_default(); - } else { - col.get_data().push_back(result); - } - } - } }; template @@ -256,6 +225,23 @@ class AggregateFunctionPercentileApproxTwoParams_OLDER : public AggregateFunctio this->data(place).add(sources.get_element(row_num), quantile.get_element(row_num)); } } + + DataTypePtr get_return_type() const override { + return make_nullable(std::make_shared()); + } + + void insert_result_into(ConstAggregateDataPtr __restrict place, IColumn& to) const override { + auto& nullable_column = assert_cast(to); + double result = AggregateFunctionPercentileApprox::data(place).get(); + + if (std::isnan(result)) { + nullable_column.insert_default(); + } else { + auto& col = assert_cast(nullable_column.get_nested_column()); + col.get_data().push_back(result); + nullable_column.get_null_map_data().push_back(0); + } + } }; class AggregateFunctionPercentileApproxTwoParams : public AggregateFunctionPercentileApprox { @@ -271,6 +257,19 @@ class AggregateFunctionPercentileApproxTwoParams : public AggregateFunctionPerce this->data(place).init(); this->data(place).add(sources.get_element(row_num), quantile.get_element(row_num)); } + + DataTypePtr get_return_type() const override { return std::make_shared(); } + + void insert_result_into(ConstAggregateDataPtr __restrict place, IColumn& to) const override { + auto& col = assert_cast(to); + double result = AggregateFunctionPercentileApprox::data(place).get(); + + if (std::isnan(result)) { + col.insert_default(); + } else { + col.get_data().push_back(result); + } + } }; template @@ -319,6 +318,23 @@ class AggregateFunctionPercentileApproxThreeParams_OLDER this->data(place).add(sources.get_element(row_num), quantile.get_element(row_num)); } } + + DataTypePtr get_return_type() const override { + return make_nullable(std::make_shared()); + } + + void insert_result_into(ConstAggregateDataPtr __restrict place, IColumn& to) const override { + auto& nullable_column = assert_cast(to); + double result = AggregateFunctionPercentileApprox::data(place).get(); + + if (std::isnan(result)) { + nullable_column.insert_default(); + } else { + auto& col = assert_cast(nullable_column.get_nested_column()); + col.get_data().push_back(result); + nullable_column.get_null_map_data().push_back(0); + } + } }; class AggregateFunctionPercentileApproxThreeParams : public AggregateFunctionPercentileApprox { @@ -337,6 +353,19 @@ class AggregateFunctionPercentileApproxThreeParams : public AggregateFunctionPer this->data(place).init(compression.get_element(row_num)); this->data(place).add(sources.get_element(row_num), quantile.get_element(row_num)); } + + DataTypePtr get_return_type() const override { return std::make_shared(); } + + void insert_result_into(ConstAggregateDataPtr __restrict place, IColumn& to) const override { + auto& col = assert_cast(to); + double result = AggregateFunctionPercentileApprox::data(place).get(); + + if (std::isnan(result)) { + col.insert_default(); + } else { + col.get_data().push_back(result); + } + } }; template @@ -390,6 +419,23 @@ class AggregateFunctionPercentileApproxWeightedThreeParams_OLDER quantile.get_element(row_num)); } } + + DataTypePtr get_return_type() const override { + return make_nullable(std::make_shared()); + } + + void insert_result_into(ConstAggregateDataPtr __restrict place, IColumn& to) const override { + auto& nullable_column = assert_cast(to); + double result = AggregateFunctionPercentileApprox::data(place).get(); + + if (std::isnan(result)) { + nullable_column.insert_default(); + } else { + auto& col = assert_cast(nullable_column.get_nested_column()); + col.get_data().push_back(result); + nullable_column.get_null_map_data().push_back(0); + } + } }; class AggregateFunctionPercentileApproxWeightedThreeParams @@ -411,6 +457,19 @@ class AggregateFunctionPercentileApproxWeightedThreeParams this->data(place).add_with_weight(sources.get_element(row_num), weight.get_element(row_num), quantile.get_element(row_num)); } + + DataTypePtr get_return_type() const override { return std::make_shared(); } + + void insert_result_into(ConstAggregateDataPtr __restrict place, IColumn& to) const override { + auto& col = assert_cast(to); + double result = AggregateFunctionPercentileApprox::data(place).get(); + + if (std::isnan(result)) { + col.insert_default(); + } else { + col.get_data().push_back(result); + } + } }; template @@ -467,6 +526,23 @@ class AggregateFunctionPercentileApproxWeightedFourParams_OLDER quantile.get_element(row_num)); } } + + DataTypePtr get_return_type() const override { + return make_nullable(std::make_shared()); + } + + void insert_result_into(ConstAggregateDataPtr __restrict place, IColumn& to) const override { + auto& nullable_column = assert_cast(to); + double result = AggregateFunctionPercentileApprox::data(place).get(); + + if (std::isnan(result)) { + nullable_column.insert_default(); + } else { + auto& col = assert_cast(nullable_column.get_nested_column()); + col.get_data().push_back(result); + nullable_column.get_null_map_data().push_back(0); + } + } }; class AggregateFunctionPercentileApproxWeightedFourParams @@ -489,6 +565,19 @@ class AggregateFunctionPercentileApproxWeightedFourParams this->data(place).add_with_weight(sources.get_element(row_num), weight.get_element(row_num), quantile.get_element(row_num)); } + + DataTypePtr get_return_type() const override { return std::make_shared(); } + + void insert_result_into(ConstAggregateDataPtr __restrict place, IColumn& to) const override { + auto& col = assert_cast(to); + double result = AggregateFunctionPercentileApprox::data(place).get(); + + if (std::isnan(result)) { + col.insert_default(); + } else { + col.get_data().push_back(result); + } + } }; template