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
