Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/11381 )
Change subject: KUDU-2566: Enhance rowset tree pruning and stop coping string
while querying
......................................................................
Patch Set 6:
(6 comments)
I'll defer to Will; didn't review in detail.
http://gerrit.cloudera.org:8080/#/c/11381/6/src/kudu/tablet/rowset_tree-test.cc
File src/kudu/tablet/rowset_tree-test.cc:
http://gerrit.cloudera.org:8080/#/c/11381/6/src/kudu/tablet/rowset_tree-test.cc@251
PS6, Line 251: SeedRandom();
Don't need to SeedRandom() twice in a test.
http://gerrit.cloudera.org:8080/#/c/11381/6/src/kudu/tablet/rowset_tree-test.cc@283
PS6, Line 283: /*
Should either fix this test case, or remove it.
http://gerrit.cloudera.org:8080/#/c/11381/6/src/kudu/tablet/rowset_tree.h
File src/kudu/tablet/rowset_tree.h:
http://gerrit.cloudera.org:8080/#/c/11381/6/src/kudu/tablet/rowset_tree.h@96
PS6, Line 96: // [lower_bound, upper_bound)
Nit: got a tab here.
http://gerrit.cloudera.org:8080/#/c/11381/6/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:
http://gerrit.cloudera.org:8080/#/c/11381/6/src/kudu/tablet/tablet.cc@1774
PS6, Line 1774: boost::optional<Slice> lower_bound = boost::none;
: boost::optional<Slice> upper_bound = boost::none;
: if (spec->lower_bound_key()) {
: lower_bound = spec->lower_bound_key()->encoded_key();
: }
: if (spec->exclusive_upper_bound_key()) {
: upper_bound =
spec->exclusive_upper_bound_key()->encoded_key();
: }
Nit: combine using ternaries:
boost::optional<Slice> lower_bound = spec->lower_bound_key() ?
spec->lower_bound_key()->encoded_key() : boost::none;
http://gerrit.cloudera.org:8080/#/c/11381/6/src/kudu/util/interval_tree-inl.h
File src/kudu/util/interval_tree-inl.h:
http://gerrit.cloudera.org:8080/#/c/11381/6/src/kudu/util/interval_tree-inl.h@402
PS6, Line 402: interval
Nit: 'intervals' was correct; this deals with multiple intervals.
http://gerrit.cloudera.org:8080/#/c/11381/6/src/kudu/util/interval_tree.h
File src/kudu/util/interval_tree.h:
http://gerrit.cloudera.org:8080/#/c/11381/6/src/kudu/util/interval_tree.h@145
PS6, Line 145: void FindIntersectingInterval(const QueryPointType
&lower_bound,
: const QueryPointType
&upper_bound,
Nit: const QueryPointType&
(no space between the type and the ampersand)
--
To view, visit http://gerrit.cloudera.org:8080/11381
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e34b4169f93f3519f694c9bf86ca5295c318e79
Gerrit-Change-Number: 11381
Gerrit-PatchSet: 6
Gerrit-Owner: helifu <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <[email protected]>
Gerrit-Comment-Date: Mon, 10 Sep 2018 20:54:14 +0000
Gerrit-HasComments: Yes