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

Reply via email to