siddharthteotia commented on a change in pull request #5635:
URL: https://github.com/apache/incubator-pinot/pull/5635#discussion_r447152977
##########
File path:
pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java
##########
@@ -750,6 +753,136 @@ public void testSelectionTransformFunction() {
pinotQuery.getFilterExpression().getFunctionCall().getOperands().get(1).getLiteral().getStringValue(),
"v1");
}
+ @Test
+ public void testOrderByQueryCompilation() {
+ PinotQuery2BrokerRequestConverter converter = new
PinotQuery2BrokerRequestConverter();
+
+ // both group by and order by in selection
+ String sql = "SELECT foo, COUNT(*) FROM myTable GROUP BY foo ORDER BY foo";
+ PinotQuery pinotQuery = CalciteSqlParser.compileToPinotQuery(sql);
+ BrokerRequest brokerRequest = converter.convert(pinotQuery);
+ List<AggregationInfo> aggregationInfos =
brokerRequest.getAggregationsInfo();
+ Assert.assertEquals(aggregationInfos.size(), 1);
+ Assert.assertEquals(aggregationInfos.get(0).getAggregationType(),
AggregationFunctionType.COUNT.toString());
+ Assert.assertEquals(aggregationInfos.get(0).getExpressions().get(0), "*");
+ List<SelectionSort> orderBy = brokerRequest.getOrderBy();
+ Assert.assertEquals(orderBy.size(), 1);
+ Assert.assertEquals(orderBy.get(0).getColumn(), "foo");
+
+ sql = "SELECT foo, COUNT(*) FROM myTable GROUP BY foo ORDER BY COUNT(*)";
+ pinotQuery = CalciteSqlParser.compileToPinotQuery(sql);
+ brokerRequest = converter.convert(pinotQuery);
+ aggregationInfos = brokerRequest.getAggregationsInfo();
+ Assert.assertEquals(aggregationInfos.size(), 1);
+ Assert.assertEquals(aggregationInfos.get(0).getAggregationType(),
AggregationFunctionType.COUNT.toString());
+ Assert.assertEquals(aggregationInfos.get(0).getExpressions().get(0), "*");
+ orderBy = brokerRequest.getOrderBy();
+ Assert.assertEquals(orderBy.size(), 1);
+ Assert.assertEquals(orderBy.get(0).getColumn(), "count(*)");
+
+ sql = "SELECT foo FROM myTable GROUP BY foo ORDER BY foo";
+ pinotQuery = CalciteSqlParser.compileToPinotQuery(sql);
+ brokerRequest = converter.convert(pinotQuery);
+ Assert.assertNull(brokerRequest.getAggregationsInfo());
+ orderBy = brokerRequest.getOrderBy();
+ Assert.assertEquals(orderBy.size(), 1);
+ Assert.assertEquals(orderBy.get(0).getColumn(), "foo");
+
+ // aggregation not provided in selection
+ sql = "SELECT foo FROM myTable GROUP BY foo ORDER BY COUNT(*)";
+ pinotQuery = CalciteSqlParser.compileToPinotQuery(sql);
+ brokerRequest = converter.convert(pinotQuery);
+ aggregationInfos = brokerRequest.getAggregationsInfo();
+ Assert.assertEquals(aggregationInfos.size(), 1);
+ Assert.assertEquals(aggregationInfos.get(0).getAggregationType(),
AggregationFunctionType.COUNT.toString());
+ Assert.assertEquals(aggregationInfos.get(0).getExpressions().get(0), "*");
+ orderBy = brokerRequest.getOrderBy();
+ Assert.assertEquals(orderBy.size(), 1);
+ Assert.assertEquals(orderBy.get(0).getColumn(), "count(*)");
+
+ // group by not provided in selection
+ sql = "SELECT COUNT(*) FROM myTable GROUP BY foo ORDER BY foo";
Review comment:
I thought at some point we had made a decision that SQL GROUP BY would
require the user to explicitly specify the GROUP BY columns in the select list
and we were ok with users changing their queries to add group by columns to the
select list. I know PQL doesn't have that restriction. Are we relaxing this
restriction on SQL side as well?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
