benWize commented on a change in pull request #16200:
URL: https://github.com/apache/beam/pull/16200#discussion_r776039125
##########
File path:
sdks/java/extensions/sql/zetasql/src/main/java/org/apache/beam/sdk/extensions/sql/zetasql/translation/AggregateScanConverter.java
##########
@@ -148,24 +149,27 @@ private LogicalProject
convertAggregateScanInputScanToLogicalProject(
// aggregation?
ResolvedAggregateFunctionCall aggregateFunctionCall =
((ResolvedAggregateFunctionCall) resolvedComputedColumn.getExpr());
- if (aggregateFunctionCall.getArgumentList() != null
- && aggregateFunctionCall.getArgumentList().size() == 1) {
- ResolvedExpr resolvedExpr =
aggregateFunctionCall.getArgumentList().get(0);
-
- // TODO: assume aggregate function's input is either a ColumnRef or a
cast(ColumnRef).
- // TODO: user might use multiple CAST so we need to handle this rare
case.
- projects.add(
- getExpressionConverter()
- .convertRexNodeFromResolvedExpr(
- resolvedExpr,
- node.getInputScan().getColumnList(),
- input.getRowType().getFieldList(),
- ImmutableMap.of()));
-
fieldNames.add(getTrait().resolveAlias(resolvedComputedColumn.getColumn()));
- } else if (aggregateFunctionCall.getArgumentList() != null
- && aggregateFunctionCall.getArgumentList().size() > 1) {
- throw new IllegalArgumentException(
- aggregateFunctionCall.getFunction().getName() + " has more than
one argument.");
+ ImmutableList<ResolvedExpr> argumentList =
+ ImmutableList.copyOf(aggregateFunctionCall.getArgumentList());
+ if (argumentList != null && argumentList.size() >= 1) {
Review comment:
I tried throwing an error for those cases, but it seems that some
functions as `COUNT(*)` (some tests failed) have empty `argumentList` and it's
expected to fall outside the if.
--
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]
