Skip to content

Commit 5566e93

Browse files
zhangstar333dataroaring
authored andcommitted
[Bug](compatibility) fix covar_samp function coredump when upgrade (#41023)
## Proposed changes in branch-2.1, the function covar_samp mode is PropagateNullable eg: select covar_samp(non_nullable(x), y) from test_covar_samp; x, y has nullable, and return is nullable and now the column of non_nullable(x), y is nullable should check is nullable alone. the fixed in branch-21 #39943 <!--Describe your changes.-->
1 parent 3d45cd5 commit 5566e93

File tree

3 files changed

+46
-13
lines changed

3 files changed

+46
-13
lines changed

be/src/vec/aggregate_functions/aggregate_function_covar.h

+39-10
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717

1818
#pragma once
1919

20+
#include <glog/logging.h>
21+
2022
#include "agent/be_exec_version_manager.h"
2123
#define POP true
2224
#define NOTPOP false
@@ -138,13 +140,22 @@ struct PopData : Data {
138140
template <typename T, typename Data>
139141
struct SampData_OLDER : Data {
140142
void insert_result_into(IColumn& to) const {
141-
ColumnNullable& nullable_column = assert_cast<ColumnNullable&>(to);
142-
if (this->count == 1 || this->count == 0) {
143-
nullable_column.insert_default();
143+
if (to.is_nullable()) {
144+
ColumnNullable& nullable_column = assert_cast<ColumnNullable&>(to);
145+
if (this->count == 1 || this->count == 0) {
146+
nullable_column.insert_default();
147+
} else {
148+
auto& col = assert_cast<ColumnFloat64&>(nullable_column.get_nested_column());
149+
col.get_data().push_back(this->get_samp_result());
150+
nullable_column.get_null_map_data().push_back(0);
151+
}
144152
} else {
145-
auto& col = assert_cast<ColumnFloat64&>(nullable_column.get_nested_column());
146-
col.get_data().push_back(this->get_samp_result());
147-
nullable_column.get_null_map_data().push_back(0);
153+
auto& col = assert_cast<ColumnFloat64&>(to);
154+
if (this->count == 1 || this->count == 0) {
155+
col.insert_default();
156+
} else {
157+
col.get_data().push_back(this->get_samp_result());
158+
}
148159
}
149160
}
150161
static DataTypePtr get_return_type() {
@@ -195,12 +206,30 @@ class AggregateFunctionSampCovariance
195206
this->data(place).add(columns[0], columns[1], row_num);
196207
} else {
197208
if constexpr (is_nullable) { //this if check could remove with old function
209+
// nullable means at least one child is null.
210+
// so here, maybe JUST ONE OF ups is null. so nullptr perhaps in ..._x or ..._y!
198211
const auto* nullable_column_x = check_and_get_column<ColumnNullable>(columns[0]);
199212
const auto* nullable_column_y = check_and_get_column<ColumnNullable>(columns[1]);
200-
if (!nullable_column_x->is_null_at(row_num) &&
201-
!nullable_column_y->is_null_at(row_num)) {
202-
this->data(place).add(&nullable_column_x->get_nested_column(),
203-
&nullable_column_y->get_nested_column(), row_num);
213+
214+
if (nullable_column_x && nullable_column_y) { // both nullable
215+
if (!nullable_column_x->is_null_at(row_num) &&
216+
!nullable_column_y->is_null_at(row_num)) {
217+
this->data(place).add(&nullable_column_x->get_nested_column(),
218+
&nullable_column_y->get_nested_column(), row_num);
219+
}
220+
} else if (nullable_column_x) { // x nullable
221+
if (!nullable_column_x->is_null_at(row_num)) {
222+
this->data(place).add(&nullable_column_x->get_nested_column(), columns[1],
223+
row_num);
224+
}
225+
} else if (nullable_column_y) { // y nullable
226+
if (!nullable_column_y->is_null_at(row_num)) {
227+
this->data(place).add(columns[0], &nullable_column_y->get_nested_column(),
228+
row_num);
229+
}
230+
} else {
231+
throw Exception(ErrorCode::INTERNAL_ERROR,
232+
"Nullable function {} get non-nullable columns!", get_name());
204233
}
205234
} else {
206235
this->data(place).add(columns[0], columns[1], row_num);

regression-test/data/nereids_function_p0/agg_function/test_covar_samp.out

+6-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
-- This file is automatically generated. You should know what you did if you want to edit this
22
-- !sql --
3-
1
3+
1.0
44

55
-- !sql --
66
-1.5
@@ -12,4 +12,8 @@
1212
4.5
1313

1414
-- !sql --
15-
1.666667
15+
1.666666666666666
16+
17+
-- !notnull3 --
18+
1.666666666666666
19+

regression-test/suites/nereids_function_p0/agg_function/test_covar_samp.groovy

+1-1
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,6 @@ suite("test_covar_samp") {
8585
(4, 4, 4)
8686
"""
8787
qt_sql "select covar_samp(x,y) from test_covar_samp"
88-
88+
qt_notnull3 "select covar_samp(non_nullable(x), y) from test_covar_samp"
8989
sql """ DROP TABLE IF EXISTS test_covar_samp """
9090
}

0 commit comments

Comments
 (0)