zclllyybb commented on code in PR #58736:
URL: https://github.com/apache/doris/pull/58736#discussion_r2618819396
##########
regression-test/suites/query_p0/sql_functions/aggregate_functions/test_aggregate_all_functions2.groovy:
##########
Review Comment:
再加一个有group by分组的case。
##########
be/src/vec/aggregate_functions/aggregate_function_min_max_by.h:
##########
@@ -124,40 +127,49 @@ template <typename KT>
struct AggregateFunctionMinMaxByBaseData {
Review Comment:
add a cooment to explain why we need two layer of MinMaxByDate
##########
be/src/vec/aggregate_functions/aggregate_function_min_max_by.h:
##########
@@ -124,40 +127,49 @@ template <typename KT>
struct AggregateFunctionMinMaxByBaseData {
protected:
std::unique_ptr<MaxMinValueBase> value;
- KT key;
+ std::unique_ptr<KT> key;
Review Comment:
seems not need to wrap with uptr?
##########
be/src/vec/aggregate_functions/aggregate_function_min_max_by.h:
##########
Review Comment:
the first template argument is always `AggregateFunctionsMinMaxBy`? could
remove it
##########
be/src/vec/aggregate_functions/aggregate_function_min_max_by.h:
##########
Review Comment:
and combine `min/max_by.cpp` to one file
--
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]
