[kudu-CR] [c++-client]: minimal changes to support tables with non-covering range partitions

["Adar Dembo (Code Review)" <ge...@cloudera.org>]

Adar Dembo has posted comments on this change.

Change subject: [c++-client]: minimal changes to support tables with non-covering range partitions
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/3177/1/src/kudu/client/meta_cache.cc
File src/kudu/client/meta_cache.cc:

Line 612:   const TabletLocationsPB& tablet0 = rpc.resp().tablet_locations(0);
I always thought it was somewhat dangerous to expect any kind of order out of the tablets in response. Since we're changing this behavior, maybe we can avoid that now?


Line 618:     if (rpc.is_exact_lookup() || rpc.resp().tablet_locations().size() <= 1) {
<= 1? Given that we're here, why bother checking for 0?


http://gerrit.cloudera.org:8080/#/c/3177/1/src/kudu/common/partition_pruner.h
File src/kudu/common/partition_pruner.h:

Line 62:   // Used for testing.
Not true anymore. Update? And what was unsafe/weird about this function to begin with that it was restricted to tests?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib25b7a57b14b3d1e4e6dca75b88dad7c19ba7565
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes