HappenLee commented on code in PR #67805:
URL: https://github.com/apache/doris/pull/67805#discussion_r3979588708
##########
be/src/exprs/aggregate/aggregate_function_collect.h:
##########
@@ -444,6 +445,18 @@ class AggregateFunctionCollect final
Arena& arena) const override {
auto& data = this->data(place);
const auto& rhs_data = this->data(rhs);
+ if constexpr (HasLimit) {
+ if (rhs_data.max_size == -1) {
Review Comment:
Negative limits have no documented SQL meaning; the existing behavior that
collects values under a negative limit predates this PR. We will treat states
with negative limits as non-contributing during merge and change the RHS check
from `rhs_data.max_size == -1` to `rhs_data.max_size < 0`.
In the current `_merge`/`_union` execution path, the destination is freshly
created, and each serialized input is deserialized into a temporary RHS before
merging. A negative-limit RHS is therefore skipped before it can populate or
configure the destination. The destination remains uninitialized (`-1`) until
it adopts a nonnegative limit, after which the existing compatibility check
applies. This path does not use a state populated by raw `add()` as the merge
destination, so the hypothetical negative-limit LHS does not arise here.
This deliberately ignores negative-limit states rather than preserving their
previously unchecked contents. We will include the change in the consolidated
follow-up update.
--
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]
