github-actions[bot] commented on code in PR #67439:
URL: https://github.com/apache/doris/pull/67439#discussion_r4000169163
##########
be/src/exprs/aggregate/aggregate_function_min_max.h:
##########
@@ -625,7 +625,7 @@ struct SingleValueDataComplexType {
}
bool change_if_less(const Self& to, Arena& arena) {
- if (to.has() && (!has() || column_data->compare_at(0, 0,
*to.column_data, -1) == 1)) {
+ if (to.has() && (!has() || column_data->compare_at(0, 0,
*to.column_data, 1) == 1)) {
Review Comment:
[P1] Version the semantic change before merging old aggregate states
This overload merges an already-reduced complex state, but the PR reverses
the order used to choose that sole state without creating a compatibility
boundary. Both the base and head still advertise BE exec version 13. For
example, an old v13 worker computing `max` over `[1, NULL]` and `[1, 5]`
serializes only `[1, 5]`; under this PR v13 `max` should return `[1, NULL]`,
and a new BE merging the old state cannot recover the discarded row. Stored
`max_state`/`min_state` values (and ARRAY-key `*_by` states) are likewise
accepted because these functions have no compatibility-map entry. Please
version/select the old policy for old states, or reject and require rebuilding
them, and cover mixed/persisted merges.
--
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]
