github-actions[bot] commented on code in PR #63128:
URL: https://github.com/apache/doris/pull/63128#discussion_r3216379456
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/BuiltinAggregateFunctions.java:
##########
@@ -135,8 +137,10 @@ private BuiltinAggregateFunctions() {
agg(BoolOr.class, "bool_or", "boolor_agg"),
agg(BoolAnd.class, "bool_and", "booland_agg"),
agg(BoolXor.class, "bool_xor", "boolxor_agg"),
- agg(CollectList.class, "collect_list", "group_array"),
- agg(CollectSet.class, "collect_set", "group_uniq_array"),
+ agg(CollectList.class, "collect_list_v1"),
Review Comment:
This makes the public `collect_list` and `group_array` names instantiate
`CollectListV2`, whose BE function name is `collect_list_v2`. During a rolling
upgrade, a new FE can plan `collect_list(...)` as `collect_list_v2` while some
BEs are still old and only register `collect_list`/`collect_set`; those
fragments will fail function lookup. The BE-side `collect_list_v1` aliases only
help new BEs accept new FE's V1 name and do not make old BEs understand V2.
Please gate the alias switch on BE capability/exec version or keep public
aliases on V1 until the cluster can guarantee all BEs support V2.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/types/AggStateType.java:
##########
@@ -56,6 +56,8 @@ public class AggStateType extends DataType {
.put("hist", "histogram")
.put("map_agg", "map_agg_v2")
.put("window_funnel", "window_funnel_v2")
+ .put("collect_list", "collect_list_v2")
Review Comment:
Remapping existing `AGG_STATE<collect_list(...)>` metadata from
`collect_list` to `collect_list_v2` is not format-compatible for complex types.
Before this PR, `collect_list` state for complex values serialized each element
through the JSON serde; the new V2 BE reader expects
`serialize_value_into_arena` binary bytes. When an existing table/MV with a
`collect_list` aggregate-state column is read after upgrade,
`DataType.fromCatalogType()` will construct this Nereids `AggStateType`, alias
the function to V2, and the merge/union path can deserialize old V1 bytes with
the V2 reader. Please preserve the stored function name for existing states or
add explicit version/format compatibility before aliasing aggregate-state types
to V2.
--
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]
