Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/10075 )
Change subject: KUDU-2191: Metadata Upgrade Tool ...................................................................... Patch Set 13: (2 comments) http://gerrit.cloudera.org:8080/#/c/10075/12/src/kudu/hms/hms_catalog.h File src/kudu/hms/hms_catalog.h: http://gerrit.cloudera.org:8080/#/c/10075/12/src/kudu/hms/hms_catalog.h@100 PS12, Line 100: // This method will fail if the HMS is unreachable. > In the following patch, the same method is used to retrieve non-legacy tabl OK, but how can this method know the Kudu table name of an HMS table entry? If you don't have knowledge of the storage handler, there's no single way to determine the Kudu table name, or even if it's a Kudu table. The current implementation just hard-codes the knowledge in a way that's only relevant to Impala legacy tables. In general I think this is just too specialized of a method for this API, which tends to be somewhat more generic. It'd be better if it returned a vector of the tables which match the storage handler, and let the caller do the special post-processing which is pretty specific to the calling context. http://gerrit.cloudera.org:8080/#/c/10075/13/src/kudu/tools/tool_action_hms.cc File src/kudu/tools/tool_action_hms.cc: http://gerrit.cloudera.org:8080/#/c/10075/13/src/kudu/tools/tool_action_hms.cc@88 PS13, Line 88: new_table_name = Substitute("$0.$1", default_database, table_name); This is the case of a non-Impala legacy table, right? I thought this tool assumed the table had already been renamed to be Hive-compatible via the new table rename tool? -- To view, visit http://gerrit.cloudera.org:8080/10075 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1c8488e1c7b04b8bedc31c83b1496b53bae49cb3 Gerrit-Change-Number: 10075 Gerrit-PatchSet: 13 Gerrit-Owner: Hao Hao <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Mon, 14 May 2018 22:52:24 +0000 Gerrit-HasComments: Yes
