Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/12946 )
Change subject: generic_iterators: pass rowset bounds into grouping iterators ...................................................................... Patch Set 6: Code-Review+1 (2 comments) http://gerrit.cloudera.org:8080/#/c/12946/6/src/kudu/tablet/rowset.h File src/kudu/tablet/rowset.h: http://gerrit.cloudera.org:8080/#/c/12946/6/src/kudu/tablet/rowset.h@153 PS6, Line 153: virtual Status NewRowIteratorWithBounds(const RowIteratorOptions& opts, I'm curious: why did you put the implementation in the header file? NewRowIterator() is not inlined. http://gerrit.cloudera.org:8080/#/c/12946/6/src/kudu/tablet/rowset.h@167 PS6, Line 167: std::move(encoded_bounds); nit: would be more terse to say: out->encoded_bounds = std::make_pair(std::move(lower), std::move(upper)); -- To view, visit http://gerrit.cloudera.org:8080/12946 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id1aebd84507f87c869d781e117225c37c8f15969 Gerrit-Change-Number: 12946 Gerrit-PatchSet: 6 Gerrit-Owner: Adar Dembo <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Sat, 13 Apr 2019 00:45:20 +0000 Gerrit-HasComments: Yes
