mayankshriv commented on a change in pull request #4535: Implement DISTINCT 
clause
URL: https://github.com/apache/incubator-pinot/pull/4535#discussion_r315314108
 
 

 ##########
 File path: 
pinot-common/src/main/java/org/apache/pinot/pql/parsers/pql2/ast/SelectAstNode.java
 ##########
 @@ -140,7 +144,23 @@ public void updateBrokerRequest(BrokerRequest 
brokerRequest) {
         groupBy.setTopN(_topN);
       } else {
         // Pinot quirk: default to top 10
-        groupBy.setTopN(10);
+        groupBy.setTopN(DEFAULT_RECORD_LIMIT);
+      }
+    }
+
+    List<AggregationInfo> aggregationInfo = 
brokerRequest.getAggregationsInfo();
+    if (aggregationInfo != null && aggregationInfo.size() == 1) {
+      // Since DISTINCT is implemented as a function in the 
AggregationOperator,
+      // the compiler would have ensured by now that for a DISTINCT query,
+      // there will be exactly one aggregation info -- erroneous cases would 
have
+      // already resulted in error when we updated the broker request for child
+      // nodes of this Select node
+      if 
(aggregationInfo.get(0).getAggregationType().equalsIgnoreCase("distinct")) {
 
 Review comment:
   The special casing for distinct seems like an issue to me.

----------------------------------------------------------------
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]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to