siddharthteotia commented on a change in pull request #4535: Implement DISTINCT
clause
URL: https://github.com/apache/incubator-pinot/pull/4535#discussion_r315334415
##########
File path: pinot-common/src/thrift/request.thrift
##########
@@ -155,6 +155,7 @@ struct BrokerRequest {
16: optional HavingFilterQueryMap havingFilterSubQueryMap;
17: optional query.PinotQuery pinotQuery;
18: optional list<SelectionSort> orderBy;
+ 19: optional i32 limit = 0;
Review comment:
The LIMIT is currently only supported in SELECT query path
(BrokerRequest.setSelection()) has the LIMIT info (both start and end). Reusing
this would have then led to change code down the line where existing code makes
decisions based on the fact
(1) if brokerRequest.getSelections() is not null then it is guaranteed to be
selection query
(2) if broerRequest.getAggregationInfo() is not null then it is guaranteed
to be aggregation query
If we end up reusing the LIMIT stashed away by the parser in SelectAstNode
for DISTINCT query as well then the above assumptions will break -- couple of
places where existing code is dependent on these invariants are places like
CombineService, BrokerReduceService
----------------------------------------------------------------
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:
us...@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
