Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10217 )

Change subject: KUDU-2191: HMS Metadata Consistency Check Tool
......................................................................


Patch Set 11:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/10217/8//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10217/8//COMMIT_MSG@11
PS8, Line 11: column
> column
Done


http://gerrit.cloudera.org:8080/#/c/10217/8//COMMIT_MSG@12
PS8, Line 12: report
> reports
Done


http://gerrit.cloudera.org:8080/#/c/10217/10//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10217/10//COMMIT_MSG@11
PS10, Line 11: column
> column
Done


http://gerrit.cloudera.org:8080/#/c/10217/7/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/10217/7/src/kudu/tools/kudu-tool-test.cc@1957
PS7, Line 1957:
> warning: local copy 'kudu_table_name' of the variable 'table_name' is never
Done


http://gerrit.cloudera.org:8080/#/c/10217/8/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/10217/8/src/kudu/tools/kudu-tool-test.cc@2177
PS8, Line 2177:
> schema
Done


http://gerrit.cloudera.org:8080/#/c/10217/10/src/kudu/tools/tool_action_hms.cc
File src/kudu/tools/tool_action_hms.cc:

http://gerrit.cloudera.org:8080/#/c/10217/10/src/kudu/tools/tool_action_hms.cc@130
PS10, Line 130: Status AlterLegacyKuduTables(KuduClient* kudu_client,
> It's kind of confusing to pass a const ref to a mutable pointer, perhaps ju
Done


http://gerrit.cloudera.org:8080/#/c/10217/10/src/kudu/tools/tool_action_hms.cc@376
PS10, Line 376:       } else {
> Consider just doing an OpenTable instead of TableExists + OpenTable, and ch
Done


http://gerrit.cloudera.org:8080/#/c/10217/10/src/kudu/tools/tool_action_hms.cc@394
PS10, Line 394:   }
> Do legacy tables have a table ID param?
No, it doesn't. I was thinking add it here so later the fix tool can take care 
of it. But after more thoughts, I think the best way to fix this is to run 
upgrade tool. So removed the logic here.


http://gerrit.cloudera.org:8080/#/c/10217/10/src/kudu/tools/tool_action_hms.cc@399
PS10, Line 399: ables(&ta
> 'are not present'
Done


http://gerrit.cloudera.org:8080/#/c/10217/10/src/kudu/tools/tool_action_hms.cc@403
PS10, Line 403:       shared_ptr<KuduTable> kudu_table;
> move hms_tables, if this is the last use of it.
Done



--
To view, visit http://gerrit.cloudera.org:8080/10217
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3495e9c755571b85beaecca849f83457b592ee90
Gerrit-Change-Number: 10217
Gerrit-PatchSet: 11
Gerrit-Owner: Hao Hao <hao....@cloudera.com>
Gerrit-Reviewer: Dan Burkert <danburk...@apache.org>
Gerrit-Reviewer: Hao Hao <hao....@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Tue, 19 Jun 2018 22:35:35 +0000
Gerrit-HasComments: Yes

Reply via email to