[kudu-CR] [c++-client] fix KuduScanTokenBuilder token generation bugs
Dan Burkert has submitted this change and it was merged. Change subject: [c++-client] fix KuduScanTokenBuilder token generation bugs .. [c++-client] fix KuduScanTokenBuilder token generation bugs This commit fixes two critical issues in the KuduScanTokenBuilder implementation: 1. Column predicates are now correctly carried through to the scan token. Prior testing didn't catch this because the predicates were being transformed into PK bounds, which have always worked correctly. This is only an issue on the serialization side, so it doesn't affect scan tokens generated by the Java client and deserialized by the C++ client. 2. Token building now works on tables with non-covering range partitioned tables. The fix is mostly copy/paste from scanner-internal, which is very similar. flex_partitioning-itest has been extended to check scan tokens, and the scan token unit tests have been updated. I also added a unit test for issue 1 on the Java side to add some confidence that the Java side is not affected. Change-Id: Iff3ec3e2399b191c71595c96212471b1e21c7446 Reviewed-on: http://gerrit.cloudera.org:8080/4007 Reviewed-by: Adar DemboTested-by: Kudu Jenkins --- M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java M src/kudu/client/scan_token-internal.cc M src/kudu/client/scan_token-test.cc M src/kudu/integration-tests/flex_partitioning-itest.cc 4 files changed, 265 insertions(+), 37 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Iff3ec3e2399b191c71595c96212471b1e21c7446 Gerrit-PatchSet: 4 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-Reviewer: Will Berkeley
[kudu-CR] [c++-client] fix KuduScanTokenBuilder token generation bugs
Kudu Jenkins has posted comments on this change. Change subject: [c++-client] fix KuduScanTokenBuilder token generation bugs .. Patch Set 3: Build Started http://104.196.14.100/job/kudu-gerrit/2955/ -- To view, visit http://gerrit.cloudera.org:8080/4007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iff3ec3e2399b191c71595c96212471b1e21c7446 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: No
[kudu-CR] [c++-client] fix KuduScanTokenBuilder token generation bugs
Adar Dembo has posted comments on this change. Change subject: [c++-client] fix KuduScanTokenBuilder token generation bugs .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/4007/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java: Line 341: private int countScanTokenRows (List tokens) throws Exception { Nit: countScanTokenRows( http://gerrit.cloudera.org:8080/#/c/4007/2/src/kudu/integration-tests/flex_partitioning-itest.cc File src/kudu/integration-tests/flex_partitioning-itest.cc: Line 268: void CheckScanWithColumnPredicate(Slice col_name, int lower, int upper); Should probably mention that this Check() variant calls the new one. Line 367: LOG(WARNING) << "Checking scan tokens: " << col_name << " [" << lower << ", " << upper << "]"; Just for debugging? -- To view, visit http://gerrit.cloudera.org:8080/4007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iff3ec3e2399b191c71595c96212471b1e21c7446 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: Yes
[kudu-CR] [c++-client] fix KuduScanTokenBuilder token generation bugs
Hello Adar Dembo, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4007 to look at the new patch set (#2). Change subject: [c++-client] fix KuduScanTokenBuilder token generation bugs .. [c++-client] fix KuduScanTokenBuilder token generation bugs This commit fixes two critical issues in the KuduScanTokenBuilder implementation: 1. Column predicates are now correctly carried through to scan token. Prior testing didn't catch this because the predicates were being transformed into PK bounds, which have always worked correctly. This is only an issue on the serialization side, so it doesn't affect scan tokens generated by the Java client and deserialized by the C++ client. 2. Token building now works on tables with non-covering range partitioned tables. The fix is mostly copy/paste from scanner-internal, which is very similar. flex_partitioning-itest has been extended to check scan tokens, and the scan token unit tests have been updated. I also added a unit test for issue 1 on the Java side to add some confidence that the Java side is not affected. Change-Id: Iff3ec3e2399b191c71595c96212471b1e21c7446 --- M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java M src/kudu/client/scan_token-internal.cc M src/kudu/client/scan_token-test.cc M src/kudu/integration-tests/flex_partitioning-itest.cc 4 files changed, 267 insertions(+), 37 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/4007/2 -- To view, visit http://gerrit.cloudera.org:8080/4007 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iff3ec3e2399b191c71595c96212471b1e21c7446 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley