somandal commented on PR #10845: URL: https://github.com/apache/pinot/pull/10845#issuecomment-1592272566
> thank you for the contribution. @vvivekiyer and @somandal > > could i ask a generic question what's the relationship between this PR and #10846? i guess specifically how do i review them? should i download both, cherry-pick/merge together and try at the same time? please share your thoughts. many thanks! @walterddr yes these two PRs should ideally be reviewed together. https://github.com/apache/pinot/pull/10846 is the planner side changes necessary for this feature. Keep in mind that we haven't yet added support for function arguments as literals etc which are heavily used for some of our aggregation functions (and this will be done as part of phase 2). Merging together may not be enough, since this PR creates a new aggregation operator. A small code change will be required to call the aggregation operator in this PR rather than the existing `AggregateOperator`. Hope this makes sense, let us know if you have any more questions -- 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. To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
