[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #6880: Fix validation logic for DISTINCT queries

[GitBox <gi...@apache.org>]

Jackie-Jiang commented on a change in pull request #6880:
URL: https://github.com/apache/incubator-pinot/pull/6880#discussion_r626900791



##########
File path: pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java
##########
@@ -1706,11 +1707,14 @@ static void validateRequest(PinotQuery pinotQuery, int queryResponseLimit) {
         }
         List<Expression> orderByList = pinotQuery.getOrderByList();
         if (orderByList != null) {
+          List<Expression> selectOperands = selectList.get(0).getFunctionCall().getOperands();
+          List<Expression> orderByOperands = new LinkedList<>();

Review comment:
       No need to create this extra list, doing per-value check should be better without creating extra garbage

##########
File path: pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java
##########
@@ -1706,11 +1707,14 @@ static void validateRequest(PinotQuery pinotQuery, int queryResponseLimit) {
         }
         List<Expression> orderByList = pinotQuery.getOrderByList();
         if (orderByList != null) {
+          List<Expression> selectOperands = selectList.get(0).getFunctionCall().getOperands();

Review comment:
       ```suggestion
             List<Expression> distinctExpressions = function.getOperands();
   ```

##########
File path: pinot-broker/src/test/java/org/apache/pinot/broker/requesthandler/QueryValidationTest.java
##########
@@ -146,4 +179,9 @@ private void testUnsupportedSQLQuery(String query, String errorMessage) {
       Assert.assertEquals(errorMessage, e.getMessage());
     }
   }
+
+  private void testSupportedSQLQuery(String query) {
+      PinotQuery pinotQuery = CalciteSqlParser.compileToPinotQuery(query);

Review comment:
       (Format) Wrong indentation




-- 
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org