github-actions[bot] commented on code in PR #63605:
URL: https://github.com/apache/doris/pull/63605#discussion_r3302409713
##########
be/src/exprs/aggregate/aggregate_function_stddev.h:
##########
@@ -144,17 +134,12 @@ struct PopData : BaseData<T, is_stddev>, Name {
template <PrimitiveType T, typename Name, bool is_stddev>
struct SampData : BaseData<T, is_stddev>, Name {
- using ColVecResult = std::conditional_t<is_decimal(T), ColumnDecimal128V2,
ColumnFloat64>;
void insert_result_into(IColumn& to) const {
- auto& col = assert_cast<ColVecResult&>(to);
+ auto& col = assert_cast<ColumnFloat64&>(to);
if (this->count == 1 || this->count == 0) {
- col.insert_default();
+ col.get_data().push_back(std::nan(""));
} else {
- if constexpr (is_decimal(T)) {
- col.get_data().push_back(this->get_samp_result().value());
Review Comment:
This still does not implement the stated behavior. For
`VAR_SAMP`/`VARIANCE_SAMP`/`STDDEV_SAMP`, a group/window frame with one valid
input should return SQL `NULL`, but this inserts a floating-point `NaN` into a
non-null `Float64` column. That is observably different: `IS NULL`, `COALESCE`,
filters, joins, JDBC/BI clients, and downstream expressions will all see a
non-null value. The regression outputs in this PR also expect `NaN`, so they
lock in the wrong semantics instead of the PR title/description goal. Please
make these sample aggregates produce a nullable result for `count <= 1` and
update the expected files to `\N` for the one-valid-value cases.
--
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]
