From df46e3824f9f856815246a348092002e04774a0b Mon Sep 17 00:00:00 2001 From: zhangstar333 <2561612514@qq.com> Date: Thu, 19 Sep 2024 21:45:22 +0800 Subject: [PATCH 1/3] [Bug](compatibility) fix stddev_samp function coredump when upgrade --- .../aggregate_function_covar.h | 16 ++++++++++++---- .../agg_function/test_covar_samp.out | 8 ++++++-- .../agg_function/test_covar_samp.groovy | 2 +- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/be/src/vec/aggregate_functions/aggregate_function_covar.h b/be/src/vec/aggregate_functions/aggregate_function_covar.h index 9b4b1b70c1fa7f..d0b0295e6c67b3 100644 --- a/be/src/vec/aggregate_functions/aggregate_function_covar.h +++ b/be/src/vec/aggregate_functions/aggregate_function_covar.h @@ -17,6 +17,8 @@ #pragma once +#include + #include "agent/be_exec_version_manager.h" #define POP true #define NOTPOP false @@ -197,11 +199,17 @@ class AggregateFunctionSampCovariance if constexpr (is_nullable) { //this if check could remove with old function const auto* nullable_column_x = check_and_get_column(columns[0]); const auto* nullable_column_y = check_and_get_column(columns[1]); - if (!nullable_column_x->is_null_at(row_num) && - !nullable_column_y->is_null_at(row_num)) { - this->data(place).add(&nullable_column_x->get_nested_column(), - &nullable_column_y->get_nested_column(), row_num); + + bool x_is_null = (nullable_column_x && nullable_column_x->is_null_at(row_num)); + bool y_is_null = (nullable_column_y && nullable_column_y->is_null_at(row_num)); + if (x_is_null || y_is_null) { + return; // if have null value, should return directly } + const auto* column_x = + nullable_column_x ? &nullable_column_x->get_nested_column() : columns[0]; + const auto* column_y = + nullable_column_y ? &nullable_column_y->get_nested_column() : columns[1]; + this->data(place).add(column_x, column_y, row_num); } else { this->data(place).add(columns[0], columns[1], row_num); } diff --git a/regression-test/data/nereids_function_p0/agg_function/test_covar_samp.out b/regression-test/data/nereids_function_p0/agg_function/test_covar_samp.out index 728beed6cc42c2..9d0444d522face 100644 --- a/regression-test/data/nereids_function_p0/agg_function/test_covar_samp.out +++ b/regression-test/data/nereids_function_p0/agg_function/test_covar_samp.out @@ -1,6 +1,6 @@ -- This file is automatically generated. You should know what you did if you want to edit this -- !sql -- -1 +1.0 -- !sql -- -1.5 @@ -12,4 +12,8 @@ 4.5 -- !sql -- -1.666667 \ No newline at end of file +1.666666666666666 + +-- !notnull3 -- +1.666666666666666 + diff --git a/regression-test/suites/nereids_function_p0/agg_function/test_covar_samp.groovy b/regression-test/suites/nereids_function_p0/agg_function/test_covar_samp.groovy index a75a933e748364..3c17713c7ef97b 100644 --- a/regression-test/suites/nereids_function_p0/agg_function/test_covar_samp.groovy +++ b/regression-test/suites/nereids_function_p0/agg_function/test_covar_samp.groovy @@ -85,6 +85,6 @@ suite("test_covar_samp") { (4, 4, 4) """ qt_sql "select covar_samp(x,y) from test_covar_samp" - + qt_notnull3 "select covar_samp(non_nullable(x), y) from test_covar_samp" sql """ DROP TABLE IF EXISTS test_covar_samp """ } From 3c78704dfbc0821e596637a55fa110cc6293b765 Mon Sep 17 00:00:00 2001 From: zhangstar333 <2561612514@qq.com> Date: Thu, 19 Sep 2024 21:55:14 +0800 Subject: [PATCH 2/3] update --- .../aggregate_function_covar.h | 30 +++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/be/src/vec/aggregate_functions/aggregate_function_covar.h b/be/src/vec/aggregate_functions/aggregate_function_covar.h index d0b0295e6c67b3..bee3b114af438e 100644 --- a/be/src/vec/aggregate_functions/aggregate_function_covar.h +++ b/be/src/vec/aggregate_functions/aggregate_function_covar.h @@ -197,19 +197,31 @@ class AggregateFunctionSampCovariance this->data(place).add(columns[0], columns[1], row_num); } else { if constexpr (is_nullable) { //this if check could remove with old function + // nullable means at least one child is null. + // so here, maybe JUST ONE OF ups is null. so nullptr perhaps in ..._x or ..._y! const auto* nullable_column_x = check_and_get_column(columns[0]); const auto* nullable_column_y = check_and_get_column(columns[1]); - bool x_is_null = (nullable_column_x && nullable_column_x->is_null_at(row_num)); - bool y_is_null = (nullable_column_y && nullable_column_y->is_null_at(row_num)); - if (x_is_null || y_is_null) { - return; // if have null value, should return directly + if (nullable_column_x && nullable_column_y) { // both nullable + if (!nullable_column_x->is_null_at(row_num) && + !nullable_column_y->is_null_at(row_num)) { + this->data(place).add(&nullable_column_x->get_nested_column(), + &nullable_column_y->get_nested_column(), row_num); + } + } else if (nullable_column_x) { // x nullable + if (!nullable_column_x->is_null_at(row_num)) { + this->data(place).add(&nullable_column_x->get_nested_column(), columns[1], + row_num); + } + } else if (nullable_column_y) { // y nullable + if (!nullable_column_y->is_null_at(row_num)) { + this->data(place).add(columns[0], &nullable_column_y->get_nested_column(), + row_num); + } + } else { + throw Exception(ErrorCode::INTERNAL_ERROR, + "Nullable function {} get non-nullable columns!", get_name()); } - const auto* column_x = - nullable_column_x ? &nullable_column_x->get_nested_column() : columns[0]; - const auto* column_y = - nullable_column_y ? &nullable_column_y->get_nested_column() : columns[1]; - this->data(place).add(column_x, column_y, row_num); } else { this->data(place).add(columns[0], columns[1], row_num); } From 4c83283ad9bc4e66b21c6d141b2228b8a755df9a Mon Sep 17 00:00:00 2001 From: zhangstar333 <2561612514@qq.com> Date: Fri, 20 Sep 2024 11:30:24 +0800 Subject: [PATCH 3/3] update insert into result --- .../aggregate_function_covar.h | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/be/src/vec/aggregate_functions/aggregate_function_covar.h b/be/src/vec/aggregate_functions/aggregate_function_covar.h index bee3b114af438e..78a3eae5bcb4e9 100644 --- a/be/src/vec/aggregate_functions/aggregate_function_covar.h +++ b/be/src/vec/aggregate_functions/aggregate_function_covar.h @@ -140,13 +140,22 @@ struct PopData : Data { template struct SampData_OLDER : Data { void insert_result_into(IColumn& to) const { - ColumnNullable& nullable_column = assert_cast(to); - if (this->count == 1 || this->count == 0) { - nullable_column.insert_default(); + if (to.is_nullable()) { + ColumnNullable& nullable_column = assert_cast(to); + if (this->count == 1 || this->count == 0) { + nullable_column.insert_default(); + } else { + auto& col = assert_cast(nullable_column.get_nested_column()); + col.get_data().push_back(this->get_samp_result()); + nullable_column.get_null_map_data().push_back(0); + } } else { - auto& col = assert_cast(nullable_column.get_nested_column()); - col.get_data().push_back(this->get_samp_result()); - nullable_column.get_null_map_data().push_back(0); + auto& col = assert_cast(to); + if (this->count == 1 || this->count == 0) { + col.insert_default(); + } else { + col.get_data().push_back(this->get_samp_result()); + } } } static DataTypePtr get_return_type() {