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

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


Patch Set 6:

(4 comments)

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

http://gerrit.cloudera.org:8080/#/c/10217/6/src/kudu/tools/kudu-tool-test.cc@1919
PS6, Line 1919: Status CreateHmsTable(bool legacy,
Might be more straightforward to split this into two methods, since there isn't 
a lot of overlap and the arguments don't apply to all cases.


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

http://gerrit.cloudera.org:8080/#/c/10217/6/src/kudu/tools/tool_action_hms.cc@271
PS6, Line 271:   bool schema_synced = true;
Might be easier to make a copy of 'hive_table', then call PopulateTable with 
it, and compare it back to itself.  If the tables are synced then they should 
be equal.  Makes it a bit more resilient to changes to PopulateTable (like if 
we add additional fields or metadata to the entry).


http://gerrit.cloudera.org:8080/#/c/10217/6/src/kudu/tools/tool_action_hms.cc@293
PS6, Line 293: sync table
should this be 'unsynchronized tables:'


http://gerrit.cloudera.org:8080/#/c/10217/6/src/kudu/tools/tool_action_hms.cc@348
PS6, Line 348: .
remove period to be consistent



--
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: 6
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: Fri, 18 May 2018 23:10:13 +0000
Gerrit-HasComments: Yes

Reply via email to