Copilot commented on code in PR #17155:
URL: https://github.com/apache/pinot/pull/17155#discussion_r2497491544
##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionFactory.java:
##########
@@ -285,6 +287,25 @@ public static AggregationFunction
getAggregationFunction(FunctionContext functio
}
return new ListAggFunction(arguments.get(0), separator,
nullHandlingEnabled);
}
+ case LISTAGGMV: {
+ Preconditions.checkArgument(numArguments == 2 || numArguments == 3,
+ "LISTAGG_MV expects 2 or 3 arguments, got: %s. The function
can be used as "
+ + "listAggMv([distinct] expression, 'separator')",
numArguments);
+ ExpressionContext separatorExpression = arguments.get(1);
+ Preconditions.checkArgument(separatorExpression.getType() ==
ExpressionContext.Type.LITERAL,
+ "LISTAGG_MV expects the 2nd argument to be literal, got: %s.
The function can be used as "
+ + "listAggMv([distinct] expression, 'separator')",
separatorExpression.getType());
Review Comment:
The error message shows an inconsistent syntax with the distinct parameter
in brackets before the expression. It should match the actual signature:
`\"LISTAGG_MV expects the 2nd argument to be literal, got: %s. The function can
be used as listAggMv(expression, 'separator'[, distinct])\"`.
```suggestion
+ "listAggMv(expression, 'separator'[, distinct])",
numArguments);
ExpressionContext separatorExpression = arguments.get(1);
Preconditions.checkArgument(separatorExpression.getType() ==
ExpressionContext.Type.LITERAL,
"LISTAGG_MV expects the 2nd argument to be literal, got: %s.
The function can be used as "
+ "listAggMv(expression, 'separator'[, distinct])",
separatorExpression.getType());
```
##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionFactory.java:
##########
@@ -285,6 +287,25 @@ public static AggregationFunction
getAggregationFunction(FunctionContext functio
}
return new ListAggFunction(arguments.get(0), separator,
nullHandlingEnabled);
}
+ case LISTAGGMV: {
+ Preconditions.checkArgument(numArguments == 2 || numArguments == 3,
+ "LISTAGG_MV expects 2 or 3 arguments, got: %s. The function
can be used as "
+ + "listAggMv([distinct] expression, 'separator')",
numArguments);
Review Comment:
The error message incorrectly shows the distinct parameter in brackets and
in the wrong position. According to the syntax in the PR description
(`listAggMv(expression, 'separator'[, true])`), the distinct parameter should
be the third argument after the separator, not before the expression. The
message should be: `\"LISTAGG_MV expects 2 or 3 arguments, got: %s. The
function can be used as listAggMv(expression, 'separator'[, distinct])\"`.
```suggestion
+ "listAggMv(expression, 'separator'[, distinct])",
numArguments);
```
--
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]
