Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11040 )
Change subject: hms precheck tool ...................................................................... Patch Set 4: (5 comments) http://gerrit.cloudera.org:8080/#/c/11040/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11040/2//COMMIT_MSG@11 PS2, Line 11: tables > Can you add a bit explanation of what kind of issue they are? Done http://gerrit.cloudera.org:8080/#/c/11040/3/src/kudu/tools/kudu-tool-test.cc File src/kudu/tools/kudu-tool-test.cc: http://gerrit.cloudera.org:8080/#/c/11040/3/src/kudu/tools/kudu-tool-test.cc@2639 PS3, Line 2639: A.B > 'A.B!' ? Done http://gerrit.cloudera.org:8080/#/c/11040/3/src/kudu/tools/tool_action_hms.cc File src/kudu/tools/tool_action_hms.cc: http://gerrit.cloudera.org:8080/#/c/11040/3/src/kudu/tools/tool_action_hms.cc@685 PS3, Line 685: s.ok()) { : // This is not a Hive-comp > Sorry that I am not sure if I follow why conflicting names with hive-incomp This has to do with how the CatalogManager normalizes table names. It effectively skips tables whose names don't match the Hive identifier rules. I've added a reference in this comment to this other comment: https://github.com/apache/kudu/blob/master/src/kudu/master/catalog_manager.cc#L4789-L4801 http://gerrit.cloudera.org:8080/#/c/11040/3/src/kudu/tools/tool_action_hms.cc@695 PS3, Line 695: > It might be more clear if print out in the following format: I went back and forth on this, and I eventually settled on just including the conflicting table names because I think the case conflicts will be clear from the non-normalized names, and I think it may be somewhat confusing to include the normalized name if there aren't any tables that actually have that name. I've also tried to keep the word 'normalized' out of user-facing messages, since it's pretty jargony. If you feel strongly it's easy to add, though. http://gerrit.cloudera.org:8080/#/c/11040/3/src/kudu/tools/tool_action_hms.cc@754 PS3, Line 754: return ModeBuilder("hms").Description("Operate on remote Hive Metastores") > nit: Is this alphabetical ordered? Nope, didn't realize we followed that convention. Fixed. -- To view, visit http://gerrit.cloudera.org:8080/11040 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia7b23cb802b6434eba7e38443d3361ef70e6543e Gerrit-Change-Number: 11040 Gerrit-PatchSet: 4 Gerrit-Owner: Dan Burkert <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Thu, 26 Jul 2018 18:28:40 +0000 Gerrit-HasComments: Yes
