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

Reply via email to