Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11039 )

Change subject: IMPALA-7342: Add initial support for user-level permissions
......................................................................


Patch Set 8:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/11039/8/fe/src/main/java/org/apache/impala/analysis/ShowGrantRoleStmt.java
File fe/src/main/java/org/apache/impala/analysis/ShowGrantRoleStmt.java:

http://gerrit.cloudera.org:8080/#/c/11039/8/fe/src/main/java/org/apache/impala/analysis/ShowGrantRoleStmt.java@52
PS8, Line 52: params.getPrivilege().setPrincipal_id(role_.getId());
need setPrincipal_type()


http://gerrit.cloudera.org:8080/#/c/11039/8/fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java
File fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java:

http://gerrit.cloudera.org:8080/#/c/11039/8/fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java@48
PS8, Line 48: A role/user can have 0 or more privileges
I think we need to document that the difference between roles and users is the 
roles list will have all roles defined in Sentry.  The users list will only 
contain users that have privileges.  It will not represent all users in the 
system.


http://gerrit.cloudera.org:8080/#/c/11039/8/fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java@227
PS8, Line 227: .
nit: "and type."


http://gerrit.cloudera.org:8080/#/c/11039/8/fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java@252
PS8, Line 252:    * Removes a principal for a given principal type. Returns the 
removed principal or null
nit: "name and"


http://gerrit.cloudera.org:8080/#/c/11039/8/fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java@253
PS8, Line 253:    * if no principal with this name existed.
nit: "and type"



--
To view, visit http://gerrit.cloudera.org:8080/11039
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I07e0d46d2e50d35bd64ee573b5aa4b779eb9e62f
Gerrit-Change-Number: 11039
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya <[email protected]>
Gerrit-Reviewer: Adam Holley <[email protected]>
Gerrit-Reviewer: Fredy Wijaya <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Comment-Date: Wed, 25 Jul 2018 15:59:34 +0000
Gerrit-HasComments: Yes

Reply via email to