HappenLee commented on code in PR #67805:
URL: https://github.com/apache/doris/pull/67805#discussion_r3980539944
##########
be/src/exprs/aggregate/aggregate_function_ema.h:
##########
@@ -86,12 +87,17 @@ struct ExponentialMovingAverageData {
}
void merge(const ExponentialMovingAverageData& rhs) {
- double hd = half_decay != 0.0 ? half_decay : rhs.half_decay;
- if (hd == 0.0) {
+ if (rhs.half_decay == 0.0) {
return;
}
- half_decay = hd;
- merge_point(rhs, hd);
+ if (half_decay == 0.0) {
+ half_decay = rhs.half_decay;
+ } else if (UNLIKELY(half_decay != rhs.half_decay)) {
Review Comment:
Implemented locally: NaN half-decay is now unsupported. A shared check in
`ExponentialMovingAverageData::write()` and `get()` throws `INVALID_ARGUMENT`
with `exponential_moving_average half decay must not be NaN`. This rejects
newly produced NaN-configured states before they can be serialized for
subsequent merging, and also rejects direct finalization paths.
The check is deliberately at serialization/finalization boundaries rather
than in per-row `add()`. Existing `merge()` compatibility checks and
zero-half-decay semantics are unchanged. Only a NaN half-decay is rejected;
this does not prohibit NaN input values/results or change infinity handling. No
backward-compatibility handling for previously serialized NaN states is added,
as discussed for this unreleased function.
All 8 `AggregateStateParametersTest` tests passed under ASAN, including
checks that both output APIs reject NaN and that zero/one half-decays still
work. Added SQL expected-error cases for the ordinary, `_state`, and `_combine`
functions have not been run. Formatting and header-hygiene checks passed;
clang-tidy encountered pre-existing diagnostics in `core/types.h` and unchanged
aggregate code. No code has been pushed; the fixes will be submitted together.
--
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]
