HappenLee commented on code in PR #67805:
URL: https://github.com/apache/doris/pull/67805#discussion_r3979651015
##########
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) {
+ return;
+ }
+ if (data.max_size != -1) {
+ if (UNLIKELY(data.max_size != rhs_data.max_size)) {
Review Comment:
Implemented locally by adding the same `checkLegalityBeforeTypeCoercion()`
constraint as `CollectList`: when a second argument is present, `CollectSet`
requires it to be constant and otherwise throws an `AnalysisException`.
Both `StateCombinator` and `CombineCombinator` delegate this legality check
to the nested aggregate, so this also rejects varying limits in
`collect_set_state` and `collect_set_combine` before execution. The BE `add()`
path is unchanged.
Added expected-error regression cases for ordinary `collect_set`, `_state`,
and `_combine` with a limit column. FE Checkstyle passed; the new regression
cases have not been executed yet. The code is currently local and will be
pushed together with the remaining agreed review fixes.
--
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]
