HappenLee commented on code in PR #67805:
URL: https://github.com/apache/doris/pull/67805#discussion_r3992411158
##########
be/src/exprs/aggregate/aggregate_function_percentile_reservoir.h:
##########
@@ -44,7 +45,15 @@ struct QuantileReservoirSampler {
}
void merge(const QuantileReservoirSampler& rhs) {
- level = rhs.level;
+ if (rhs.data.empty()) {
Review Comment:
After discussing the intended contract, we will preserve the current
Reservoir merge behavior. A state with no non-NaN samples is intentionally
non-contributing, even if add() recorded a finite quantile level. Its level is
ignored in either merge order, including after serialization. Matching levels
are required only when both states contain non-NaN samples.
For example, merging an all-NaN state at level 0.25 with a contributing
state at level 0.75 uses the latter state's level and samples in either
direction. This is an explicit Reservoir exception to the configured-empty
behavior of exact percentile and window funnel. It does not relax FE validation
of the quantile argument: a NaN level remains invalid.
Documented this contract in the source comment in
3d605171549759cdfba0321118c21e4efef15e1c. This is a comment-only change; merge
behavior and serialized layouts are unchanged. clang-format 16.0.6, build
hygiene, and git diff --check passed. No build or runtime tests were rerun.
Resolving this thread as intended behavior.
--
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]
