Todd Lipcon has posted comments on this change.

Change subject: KUDU-1065: [java client] Flexible Partition Pruning

Patch Set 3:

Commit Message:

PS3, Line 16: hus begat 
any commit message including the phrase "Thus begat" wins extra points from me!

Line 311:    * Not for public consumption: use either predicates or primary key 
bounds instead.
given it's package-private, do you need this comment?

Line 325:    * Not for public consumption: use either predicates or primary key 
bounds instead.
File java/kudu-client/src/main/java/org/apache/kudu/client/

Line 197:     if (len > 1) {
is this 'if' really necessary? seems like if len <= 1, the len - 2 would be 
negative and the for loop woudl already skip itself
File java/kudu-client/src/main/java/org/apache/kudu/client/

PS3, Line 171:       // The range component is constrained.
             :       constrainedIndex = hashBuckets.size();
not 100% following here, can you explain further?
File java/kudu-client/src/main/java/org/apache/kudu/util/

Line 116:   public void reserve(int additional) {
I find it surprising that 'reserve' here is reserving _additional_ bytes, vs 
the std::string::reserve() function which is the absolute value. Mind renaming 
to reserveAdditional() or reserveForAppend() or something of that nature? Or 
change it to be the same as std::string?

PS3, Line 97:     for (Partition partition : partitions) {
            :       if (!pruner.shouldPrune(partition)) scannedPartitions++;
            :     }
do we expect loops like this in real life? this seems o(n^2). Should we offer a 
'prunePartitionList(List<Partition> partitions)' function?

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib27b54841d87cf854175ab8cdfa8798b337d71f9
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <>
Gerrit-Reviewer: Adar Dembo <>
Gerrit-Reviewer: Dan Burkert <>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <>
Gerrit-HasComments: Yes

Reply via email to