Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13673 )

Change subject: [IMPALA-8587] Show inherited privileges with Ranger show grant
......................................................................


Patch Set 3:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/13673/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13673/3//COMMIT_MSG@7
PS3, Line 7: [IMPALA-8587]
nit: use IMPALA-8587 format


http://gerrit.cloudera.org:8080/#/c/13673/3//COMMIT_MSG@20
PS3, Line 20: They would see no results. After this change, the user will see 
database
            : level privileges when executing the previous statement. If a user 
has
            : SELECT privilege on DATABASE and on TABLE and issues a show grant 
on
            : TABLE, they will only see the SELECT privilege for TABLE. Users 
will not
            : see multiple instances of SELECT or any other privilege type in a 
SHOW
            : GRANT statemenet.
Show what the new output looks like. It'll be much easier to understand.

We also need more explanation about this especially since this is relates to 
how the Ranger policy engine works.


http://gerrit.cloudera.org:8080/#/c/13673/3/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java
File 
fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java:

http://gerrit.cloudera.org:8080/#/c/13673/3/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java@330
PS3, Line 330:  !=
Use equals(). != or == is for reference equality. Sometimes you get lucky 
because string intern, but we shouldn't rely on that.


http://gerrit.cloudera.org:8080/#/c/13673/3/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java@329
PS3, Line 329:       for (RangerResultRow row : resultRows) {
             :         if (row.column_ != "*" && !row.column_.isEmpty()) {
             :           resourceResult.addColumnResult(row);
             :         } else if (row.table_ != "*" && !row.table_.isEmpty()) {
             :           resourceResult.addTableResult(row);
             :         } else if (row.database_ != "*" && 
!row.database_.isEmpty()) {
             :           resourceResult.addDatabaseResult(row);
             :         } else {
             :           resourceResult.addServerResult(row);
             :         }
             :       }
Can you add a comment for this logic? It's not quite clear to me what it's 
trying to do.


http://gerrit.cloudera.org:8080/#/c/13673/3/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java@422
PS3, Line 422:
nit: overly indented, use 4 spaces



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5c4c9327acb12abc12d130ef5c1ace6a08ed193c
Gerrit-Change-Number: 13673
Gerrit-PatchSet: 3
Gerrit-Owner: Austin Nobis <ano...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Jun 2019 21:08:45 +0000
Gerrit-HasComments: Yes

Reply via email to