Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15018 )
Change subject: KUDU-2973 Add semi-database support for Ranger ...................................................................... Patch Set 10: (5 comments) http://gerrit.cloudera.org:8080/#/c/15018/9//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15018/9//COMMIT_MSG@17 PS9, Line 17: case : sensitive nit: should this be updated? http://gerrit.cloudera.org:8080/#/c/15018/9/src/kudu/common/table_util.h File src/kudu/common/table_util.h: http://gerrit.cloudera.org:8080/#/c/15018/9/src/kudu/common/table_util.h@40 PS9, Line 40: Status ParseRangerTableIdentifier(const std::string& table_name, nit: Can you add a comment for this method? http://gerrit.cloudera.org:8080/#/c/15018/9/src/kudu/common/table_util.cc File src/kudu/common/table_util.cc: http://gerrit.cloudera.org:8080/#/c/15018/9/src/kudu/common/table_util.cc@30 PS9, Line 30: Name of the default database which is used in the Ranger " : "authorization context when the database name is not specified " : "in the table name. Should we note here that if users should be careful about creating tables with both 'foobar' and 'default.foobar', which will be considered to use the same table in Ranger. I don't have a good solution for proactively detect this other than doc, but wondering what others think. http://gerrit.cloudera.org:8080/#/c/15018/9/src/kudu/common/table_util.cc@44 PS9, Line 44: must be other characters after the first one, as the first period is treated as nit: not begin or end with a period ('.')? Do you think we need to have a tool to upgrade for such table names? http://gerrit.cloudera.org:8080/#/c/15018/9/src/kudu/common/table_util.cc@106 PS9, Line 106: gument(kInvalidRangerTableE If we think this is not possible, maybe remove it? -- To view, visit http://gerrit.cloudera.org:8080/15018 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I11431ff5bc75540edff56ef3d4ad384fa37d33d5 Gerrit-Change-Number: 15018 Gerrit-PatchSet: 10 Gerrit-Owner: Attila Bukor <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Anonymous Coward (314) Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 06 Feb 2020 18:26:47 +0000 Gerrit-HasComments: Yes
