Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11244 )
Change subject: IMPALA-6989: Implement SHOW GRANT USER statement ...................................................................... Patch Set 2: (1 comment) looks fine.. main question is naming consistency. http://gerrit.cloudera.org:8080/#/c/11244/2/common/thrift/Frontend.thrift File common/thrift/Frontend.thrift: http://gerrit.cloudera.org:8080/#/c/11244/2/common/thrift/Frontend.thrift@442 PS2, Line 442: ROLE_OR_USER In the refactor that added principal, you mentioned in a chat that principal may be extended to include group. If that's the case, should the name here be SHOW_GRANT_PRINCIPAL so that we avoid the alternative way to extend this to SHOW_GRANT_ROLE_OR_USER_OR_GROUP? I'm ok with this name if such an extension is not planned. also noting that the method calls favor principal in some cases, such as getPrincipalPrivilege, not getRoleOrUserPrivilege. -- To view, visit http://gerrit.cloudera.org:8080/11244 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia96fcf02d249501c471d03511c22625ae2fec225 Gerrit-Change-Number: 11244 Gerrit-PatchSet: 2 Gerrit-Owner: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Adam Holley <ahol...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com> Gerrit-Comment-Date: Fri, 17 Aug 2018 17:30:13 +0000 Gerrit-HasComments: Yes