Dan Burkert has posted comments on this change. Change subject: [java-client] implement KuduPredicate API ......................................................................
Patch Set 8: (4 comments) omitting the boolean tests, and marking the ctor internal was a mistake. The boolean constructor is converting the boolean range predicates to non-range predicates. I've added comments that hopefully clears it up. http://gerrit.cloudera.org:8080/#/c/2591/8/java/kudu-client/src/main/java/org/kududb/client/KuduPredicate.java File java/kudu-client/src/main/java/org/kududb/client/KuduPredicate.java: Line 81: * @param op the comparison operation, must be ComparisonType.EQUALITY > This line seems wrong now. Done Line 84: @VisibleForTesting > So this is not part of the public API? this was a mistake, fixed. Line 239: Float > Double? Done http://gerrit.cloudera.org:8080/#/c/2591/8/java/kudu-client/src/test/java/org/kududb/client/TestScanPredicate.java File java/kudu-client/src/test/java/org/kududb/client/TestScanPredicate.java: Line 218: Insert nullInsert = table.newInsert(); > Seems like at least the nullInsert part could be refactored. maybe, but it would save at most two lines from each test case, and handling the key value is pretty awkward. -- 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: 8 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
