walterddr commented on code in PR #11822:
URL: https://github.com/apache/pinot/pull/11822#discussion_r1364700484
##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionFactory.java:
##########
@@ -227,14 +238,40 @@ public static AggregationFunction
getAggregationFunction(FunctionContext functio
}
}
case ARRAYAGG: {
- Preconditions.checkArgument(numArguments == 2,
- "ARRAY_AGG expects 2 arguments, got: %s. The function can be
used as "
- + "arrayAgg(dataColumn, 'dataType')", numArguments);
+ Preconditions.checkArgument(numArguments >= 2,
+ "ARRAY_AGG expects 2 or 3 arguments, got: %s. The function can
be used as "
+ + "arrayAgg(dataColumn, 'dataType', ['isDistinct'])",
numArguments);
Review Comment:
we should probably consider how to parse this. b/c the proper way to express
distinct array agg is
```
SELECT key, ARRAY_AGG(DISTINCT col)
FROM tbl
GROUP BY key
```
given that our functions are not standard it is fine we do it now but good
to note this
##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionFactory.java:
##########
@@ -227,14 +238,40 @@ public static AggregationFunction
getAggregationFunction(FunctionContext functio
}
}
case ARRAYAGG: {
- Preconditions.checkArgument(numArguments == 2,
- "ARRAY_AGG expects 2 arguments, got: %s. The function can be
used as "
- + "arrayAgg(dataColumn, 'dataType')", numArguments);
+ Preconditions.checkArgument(numArguments >= 2,
+ "ARRAY_AGG expects 2 or 3 arguments, got: %s. The function can
be used as "
+ + "arrayAgg(dataColumn, 'dataType', ['isDistinct'])",
numArguments);
Review Comment:
also see:
https://www.postgresql.org/docs/16/functions-aggregate.html#FUNCTIONS-AGGREGATE
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/custom/ArrayTest.java:
##########
@@ -76,6 +76,33 @@ public void testQueries(boolean useMultiStageQueryEngine)
Assert.assertEquals(jsonNode.get("resultTable").get("rows").get(0).get(4).size(),
getCountStarResult());
}
+ @Test(dataProvider = "useBothQueryEngines")
+ public void testQueryWithDistinct(boolean useMultiStageQueryEngine)
+ throws Exception {
+ setUseMultiStageQueryEngine(useMultiStageQueryEngine);
+ String query =
+ String.format("SELECT "
+ + "arrayAgg(boolCol, 'BOOLEAN', true), "
+ + "arrayAgg(intCol, 'INT', true), "
+ + "arrayAgg(longCol, 'LONG', true), "
+ // NOTE: FLOAT array is auto converted to DOUBLE array
+ + (useMultiStageQueryEngine ? "arrayAgg(floatCol, 'DOUBLE', true),
"
Review Comment:
CC @Jackie-Jiang this is related to #11518
--
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]
