Adar Dembo has posted comments on this change.

Change subject: KUDU-1713: add a client Partitioner API
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5775/4/src/kudu/client/partitioner-internal.cc
File src/kudu/client/partitioner-internal.cc:

PS4, Line 42:   if (!table_) {
            :     return Status::InvalidArgument("null table");
            :   }
Isn't this already accounted for in the KuduPartitionerBuilder::Data 
constructor?


http://gerrit.cloudera.org:8080/#/c/5775/3/src/kudu/client/partitioner-internal.h
File src/kudu/client/partitioner-internal.h:

Line 55:   // A deferred Status result, set to non-OK if one of the builder
> because the keys we're looking up fall between partitions, so we need sorte
This was my bad in any case; I looked at PartitionRow() after I wrote this 
comment and didn't make the FindFloorOrDie() connection, which requires an 
std::map.


-- 
To view, visit http://gerrit.cloudera.org:8080/5775
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic08a078c75f15ef4200219b5260cfb99d79b72cc
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to