Copilot commented on code in PR #17155:
URL: https://github.com/apache/pinot/pull/17155#discussion_r2502023084
##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionFactory.java:
##########
@@ -268,12 +268,12 @@ public static AggregationFunction
getAggregationFunction(FunctionContext functio
}
case LISTAGG: {
Preconditions.checkArgument(numArguments == 2 || numArguments == 3,
- "LISTAGG expects 2 arguments, got: %s. The function can be
used as "
- + "listAgg([distinct] expression, 'separator')",
numArguments);
+ "LISTAGG expects 2 or 3 arguments, got: %s. The function can
be used as "
+ + "listAgg(expression, 'separator'[, distinct])",
numArguments);
ExpressionContext separatorExpression = arguments.get(1);
Preconditions.checkArgument(separatorExpression.getType() ==
ExpressionContext.Type.LITERAL,
"LISTAGG expects the 2nd argument to be literal, got: %s. The
function can be used as "
- + "listAgg([distinct] expression, 'separator')",
separatorExpression.getType());
+ + "listAgg(expression, 'separator'[, distinct])",
separatorExpression.getType());
Review Comment:
The error message should clarify that the third argument is a boolean
literal (true/false), not just 'distinct'. The syntax `[, distinct]` is
ambiguous—users might think they need to pass the word 'distinct' rather than a
boolean value.
##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionFactory.java:
##########
@@ -268,12 +268,12 @@ public static AggregationFunction
getAggregationFunction(FunctionContext functio
}
case LISTAGG: {
Preconditions.checkArgument(numArguments == 2 || numArguments == 3,
- "LISTAGG expects 2 arguments, got: %s. The function can be
used as "
- + "listAgg([distinct] expression, 'separator')",
numArguments);
+ "LISTAGG expects 2 or 3 arguments, got: %s. The function can
be used as "
+ + "listAgg(expression, 'separator'[, distinct])",
numArguments);
Review Comment:
The error message should clarify that the third argument is a boolean
literal (true/false), not just 'distinct'. The syntax `[, distinct]` is
ambiguous—users might think they need to pass the word 'distinct' rather than a
boolean value.
--
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]
