HappenLee commented on code in PR #67805:
URL: https://github.com/apache/doris/pull/67805#discussion_r3981280930
##########
be/src/exprs/aggregate/aggregate_function_group_concat.h:
##########
@@ -73,6 +74,10 @@ struct AggregateFunctionGroupConcatData {
separator = rhs.separator;
data.assign(rhs.data);
} else {
+ if (UNLIKELY(separator != rhs.separator)) {
Review Comment:
Thanks for pointing out this gap. The distinct wrapper merges and serializes
its outer argument set, then feeds the reconstructed arguments to the nested
aggregate via add() during finalization. It does not call the nested
group_concat merge(), so the separator check added here does not cover this
path.
We will defer this issue to a separate follow-up PR. Preserving support
requires the wrapper to retain and validate its configuration before combining
argument sets; alternatively, restricting AggState support needs consistent
handling across function combinators, DDL, and existing stored states. We do
not want to fold that broader change into this PR.
No implementation change for this issue is included here. Leaving this
thread unresolved to make the remaining gap explicit.
--
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]