You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2019/11/22 23:14:20 UTC

[GitHub] [incubator-pinot] npawar opened a new pull request #4851: Split BrokerReduceService code into ResultSetters

npawar opened a new pull request #4851: Split BrokerReduceService code into ResultSetters
URL: https://github.com/apache/incubator-pinot/pull/4851
 
 
   Splitting the code which sets results into BrokerResponseNative, in BrokerReduceService, into multiple ResultSetters. The work in this PR is mostly copy pasting the methods from BrokerReduceService into the right Setter, without any logic change. The only logic added, is the handling of empty dataTableMap into each setter. It was done upfront in the earlier implementation, before selecting the path based on the query.
   
   This is necessary because for the sql compliance work, we will be making each query type generate results into ResultTable. Currently this exists only for group by with order by. As we add this code for each query (selection, aggregation, distinct, group by), the BrokerReduceService reduce method will get extremely long and windy. This was attempted in https://github.com/apache/incubator-pinot/pull/4833 and it became difficult to review. Hence decided to split that PR into multiple steps
   1. ResultSetters (this PR)
   2. AggregationFunction changes
   3. ResultTable for all

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


With regards,
Apache Git Services

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