Dan Burkert has posted comments on this change. Change subject: [java-client] implement ColumnPredicate API ......................................................................
Patch Set 1: (8 comments) http://gerrit.cloudera.org:8080/#/c/2591/1//COMMIT_MSG Commit Message: Line 16: ColumnRangePredicate API. > will you do that in a follow-up? people might refer to unit tests, etc, to We only have a single unit tests using (old style) predicates, and I would prefer to keep that so that we are testing the old -> new conversion. A follow up commit will switch over the map reduce input format once scan tokens are done. http://gerrit.cloudera.org:8080/#/c/2591/1/java/kudu-client/src/main/java/org/kududb/client/ColumnPredicate.java File java/kudu-client/src/main/java/org/kududb/client/ColumnPredicate.java: Line 46: None, > style: should be all caps (https://google.github.io/styleguide/javaguide.ht Done Line 56: * the createEquality value if this is an Equality predicate. > "the createEquality value" seems a little nonsensical to me Done Line 64: * Column predicate constructor. > don't think this adds much - just use the @params I think Done Line 96: public static ColumnPredicate equality(ColumnSchema column, boolean value) { > instead of the three different sets of factory functions (lower-bound, uppe Done Line 468: * {@code ColumnPredicate} which matches the logical intersection ({@code OR}) > this should be AND, no? Done Line 540: // optimization or predicate merging. > can you clarify this a little more? if we aren't doing predicate merging, t Done Line 608: private boolean isIncrement(byte[] a, byte[] b) { > 'areConsecutive' is more readable (increment is a verb) Done -- To view, visit http://gerrit.cloudera.org:8080/2591 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icdca28139a2f4f15633cfd872e372429bad831cd Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
