924060929 commented on code in PR #12583:
URL: https://github.com/apache/doris/pull/12583#discussion_r1008811376
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/BuiltinFunctions.java:
##########
@@ -51,6 +53,11 @@ public class BuiltinFunctions implements FunctionHelper {
agg(Sum.class)
);
+ public final ImmutableList<GroupingSetsFunc> groupingSetsFunctions =
ImmutableList.of(
+ groupingSets(Grouping.class),
+ groupingSets(GroupingId.class, "grouping_id")
+ );
Review Comment:
`grouping()` and `grouping_id()` are scalar function definitely, don't use
special function list here, this will make others imitate wrongly and add more
special function list.
The list separate by the function type of top abstract level:
- scalar function
- aggregate function
- table function
They describe how many rows would be consumed and how many rows would be
produced, any function should be classified into one of them.
So I suggest rename GroupingSetsFunc to GroupingScalarFunc and extends
ScalarFunc, rename groupingSets() to groupingScalar(), and then register in the
scalar function list, without other special list.
```java
public final List<ScalarFunc> scalarFunctions = ImmutableList.builder()
.add(...) // other general functions
.add(groupingScalar(Grouping.class))
.add(groupingScalar(GroupingId.class, "groupinig_id"))
.build();
```
--
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: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
