github-actions[bot] commented on code in PR #64624:
URL: https://github.com/apache/doris/pull/64624#discussion_r3546146606
##########
be/src/exprs/aggregate/aggregate_function_null.h:
##########
@@ -606,6 +645,9 @@ class AggregateFunctionNullVariadicInline final
void add(AggregateDataPtr __restrict place, const IColumn** columns,
ssize_t row_num,
Arena& arena) const override {
+ if (this->check_always_const_col_has_null(columns)) {
Review Comment:
This still leaves non-null nullable constant arguments broken.
`_calc_argument_columns()` now feeds required const args as a top-level
`ColumnConst`, so a constant expression with nullable type, for example `SET
debug_skip_fold_constant=true; SELECT percentile_approx(kdbl, if(true, 0.5,
null)) ...`, reaches this wrapper as `ColumnConst(ColumnNullable(...))`.
`check_input_columns_type()` accepts exactly that shape below by unwrapping the
nullable data column into a const nested column, but `add()` only returns when
the const nullable value is NULL. For a non-null value it falls through and
`assert_cast`s the top-level `ColumnConst` to `ColumnNullable`, which fails in
debug and is unsafe in release. Please make the variadic add path unwrap
`ColumnConst(ColumnNullable)` the same way as the checker, or normalize
required const nullable arguments before this wrapper sees them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
