Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/10394 )
Change subject: IMPALA-110 (part 2): Refactor PartitionedAggregationNode ...................................................................... Patch Set 2: > > I think that's probably unnecessary (hopefully this shouldn't be > > too terrible to review, since the logic is entirely unchanged), > > But it sounds like there is some changes in here that aren't purely > code motion. i.e. some cleanups are in here too. so what i'm really > wondering is how can we focus the review on those parts? Maybe you > just want to add comments to indicate which code isn't a straight > copy (and class rename)? Or some other way to highlight the more > interesting parts? There's not really much cleanup, just the changes absolutely necessary to make everything work, eg. defining the Aggregator interface. All of the interesting changes are in the top commit of the branch I posted (https://github.com/twmarshall/impala/commits/agg-refactor). Happy to submit that as a review, or otherwise stage this however you think is most convenient for review. -- To view, visit http://gerrit.cloudera.org:8080/10394 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9e7bb583f54aa4add3738bde7f57cf3511ac567e Gerrit-Change-Number: 10394 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Thomas Marshall <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Mon, 18 Jun 2018 23:43:34 +0000 Gerrit-HasComments: No
