Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13521 )
Change subject: [c++] Support table rename between scan token creation and rehydration ...................................................................... Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/13521/2/src/kudu/client/client-internal.h File src/kudu/client/client-internal.h: http://gerrit.cloudera.org:8080/#/c/13521/2/src/kudu/client/client-internal.h@144 PS2, Line 144: enum class TableIdentifierType { Maybe just pass the TableIdentifierPB into these functions directly? They're all internal so it should be a clean way to encapsulate the "this string or that string" logic without new code. Too bad we can't replace the table name string with a TableIdentifierPB in the scan token protobuf though. http://gerrit.cloudera.org:8080/#/c/13521/2/src/kudu/client/client-internal.h@169 PS2, Line 169: Status GetTableSchema(KuduClient* client, : const std::string& table_name, : const MonoTime& deadline, : KuduSchema* schema, : PartitionSchema* partition_schema, : std::string* table_id, : int* num_replicas); Can we eliminate this variant? Seems like it's only used in one or two places; shouldn't be much more work for existing callers to use the new variant you added. -- To view, visit http://gerrit.cloudera.org:8080/13521 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib4d48513dff67012f26a99877b168d777d3049fd Gerrit-Change-Number: 13521 Gerrit-PatchSet: 2 Gerrit-Owner: Will Berkeley <wdberke...@gmail.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com> Gerrit-Comment-Date: Thu, 06 Jun 2019 05:29:10 +0000 Gerrit-HasComments: Yes
