yanlin-Lynn commented on a change in pull request #2021:
URL: https://github.com/apache/calcite/pull/2021#discussion_r443630098
##########
File path: core/src/main/java/org/apache/calcite/rel/externalize/RelJson.java
##########
@@ -271,7 +271,9 @@ public RelDataType toType(RelDataTypeFactory typeFactory,
Object o) {
public Object toJson(AggregateCall node) {
final Map<String, Object> map = jsonBuilder.map();
- map.put("agg", toJson(node.getAggregation()));
+ Map<String, Object> aggMap = new HashMap(toJson(node.getAggregation()));
+ aggMap.put("class", node.getAggregation().getClass().getName());
Review comment:
why new a map here, is it OK to do like this?
```
final Map<String, Object> aggMap = toJson(node.getAggregation());
aggMap.put("class", node.getAggregation().getClass().getName());
```
And you only need to add class info for UDAF, for system aggregate function,
you do not need to add class info in the map.
##########
File path: core/src/test/java/org/apache/calcite/test/MockSqlOperatorTable.java
##########
@@ -125,6 +125,18 @@ public RelDataType inferReturnType(SqlOperatorBinding
opBinding) {
}
}
+ /** "MYAGGFUNC" user-defined aggregate function. This agg function accept
one or more arguments
+ * in order to reproduce the throws of CALCITE-3929. */
+ public static class MyAggFunc extends SqlAggFunction {
Review comment:
extends `SqlUserDefinedAggFunction` might be better for your case
##########
File path: core/src/test/java/org/apache/calcite/plan/RelWriterTest.java
##########
@@ -146,8 +147,9 @@
+ " {\n"
+ " \"agg\": {\n"
+ " \"name\": \"COUNT\",\n"
- + " \"kind\": \"COUNT\",\n"
- + " \"syntax\": \"FUNCTION_STAR\"\n"
+ + " \"syntax\": \"FUNCTION_STAR\",\n"
+ + " \"class\":
\"org.apache.calcite.sql.fun.SqlCountAggFunction\",\n"
+ + " \"kind\": \"COUNT\"\n"
Review comment:
I don't think class info is needed for system aggregate function
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
