Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13535 )
Change subject: KUDU-2809 (3/6): C++ private API for diff scans ...................................................................... Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/13535/2/src/kudu/client/client.cc File src/kudu/client/client.cc: http://gerrit.cloudera.org:8080/#/c/13535/2/src/kudu/client/client.cc@1337 PS2, Line 1337: Status KuduScanner::SetDiffScan(uint64_t start_timestamp, uint64_t end_timestamp) { > Yeah, validating (start <= end) would be a good fit for config->SetDiffScan That's not done in the Java client, but I'll add it here (and to the Java client in a separate patch). http://gerrit.cloudera.org:8080/#/c/13535/2/src/kudu/client/client.cc@1737 PS2, Line 1737: data_->mutable_configuration()->SetSnapshotRaw(snapshot_timestamp); : return Status::OK(); : } > Maybe == kNoTimestamp is better (also equal to 0) Done http://gerrit.cloudera.org:8080/#/c/13535/2/src/kudu/client/client.cc@1740 PS2, Line 1740: > maybe just validate that start <= end ... also consider moving all of this Done http://gerrit.cloudera.org:8080/#/c/13535/2/src/kudu/client/scan_batch.h File src/kudu/client/scan_batch.h: http://gerrit.cloudera.org:8080/#/c/13535/2/src/kudu/client/scan_batch.h@189 PS2, Line 189: /// @return Operation result status. Return a bad Status if at least one > The user won't know the name/index here right? Can this be handled like Row Sure, though it'll require more complicated schema surgery. -- To view, visit http://gerrit.cloudera.org:8080/13535 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I03bd5e51c553b239e36fbdfc4a94131c9599851e Gerrit-Change-Number: 13535 Gerrit-PatchSet: 3 Gerrit-Owner: Adar Dembo <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Fri, 07 Jun 2019 05:51:57 +0000 Gerrit-HasComments: Yes
