siddharthteotia commented on a change in pull request #4535: Implement DISTINCT
clause
URL: https://github.com/apache/incubator-pinot/pull/4535#discussion_r315356244
##########
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 reasoning here is same as in the comment below for LIMIT. Currently the
LIMIT info is stashed away only for a selection query in
BrokerRequest.setSelection(). However for a DISTINCT query (which is internally
an aggregation based execution), we need to support LIMIT as well.
However, for this change it is probably fine to remove the check and simply
do brokerRequest.setLimit() -- the only user will be aggregation executor for
DISTINCT query so it will be harmless to other code
----------------------------------------------------------------
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]
