Todd Lipcon has posted comments on this change. Change subject: [c++ client] - Expose the schema being scanned in KuduScanBatch ......................................................................
Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/2020/4/src/kudu/client/scan_batch.h File src/kudu/client/scan_batch.h: Line 171: const KuduSchema* client_schema_; hrm, I have some worries about the perf impacts of this. keeping the RowPtrs is small makes them cheap to pass/copy by value, but the wider they get, the less they're like a simple pointer. What's the use case where you need this pointer from the RowPtr level and not from the ScanBatch level? The comments during the review seemed to indicate there isn't really an argument here except for cross-client consistency, which I dont think is that strong of a reason given it adds an unavoidable per-row perf overhead to the scan path -- To view, visit http://gerrit.cloudera.org:8080/2020 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id4276036d5ba7faf7959b4324fd013b68a08bec5 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
