Jackie-Jiang commented on a change in pull request #7972:
URL: https://github.com/apache/pinot/pull/7972#discussion_r784373637
##########
File path:
pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
##########
@@ -167,6 +170,18 @@ private static void validateGroupByClause(PinotQuery
pinotQuery)
}
}
+ private static boolean containsAggregateAndColsInSelectClause(PinotQuery
pinotQuery) {
+ int aggregateExprCount = 0;
+ int identifierExprCount = 0;
+ for (Expression expr : pinotQuery.getSelectList()) {
+ if (isAggregateExpression(expr)) {
+ aggregateExprCount++;
+ } else if (expressionContainsColumn(expr)) {
+ identifierExprCount++;
+ }
+ }
+ return identifierExprCount > 0 && aggregateExprCount > 0;
+ }
Review comment:
(minor, code format) Add empty line between functions
##########
File path:
pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
##########
@@ -167,6 +170,18 @@ private static void validateGroupByClause(PinotQuery
pinotQuery)
}
}
+ private static boolean containsAggregateAndColsInSelectClause(PinotQuery
pinotQuery) {
+ int aggregateExprCount = 0;
+ int identifierExprCount = 0;
+ for (Expression expr : pinotQuery.getSelectList()) {
+ if (isAggregateExpression(expr)) {
+ aggregateExprCount++;
+ } else if (expressionContainsColumn(expr)) {
Review comment:
I feel we can simplify the checks. Without group-by clause, the select
expressions should be either all aggregations or no aggregations (I don't think
we need to support mixing literals and aggregations)
##########
File path: pinot-common/src/test/resources/sql_queries.list
##########
@@ -820,8 +820,8 @@ select mapKey(mapField,k1) from baseballStats where
mapKey(mapField,k1) = 'v1'
SELECT count(c1), sum(c1), min(c1), max(c1), avg(c1), minmaxrange(c1),
distinctcount(c1), distinctcounthll(c1) FROM foo
SELECT distinctcountrawhll(c1), fasthll(c1), percentile90(c1),
percentileest95(c1), percentiletdigest(c1, 99) FROM foo
SELECT countmv(c1), summv(c1), minmv(c1), maxmv(c1), avgmv(c1),
minmaxrangemv(c1), distinctcountmv(c1), distinctcounthllmv(c1) FROM foo
-SELECT distinctcountrawhllmv(c1), fasthllmv(c1), percentile90mv(c1),
percentileest95mv(c1), percentiletdigestmv(c1, 99) FROM foo
+SELECT distinctcountrawhllmv(c1), percentile90mv(c1), percentileest95mv(c1),
percentiletdigestmv(c1, 99) FROM foo
Review comment:
(minor) Any specific reason changing these 2 queries?
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
