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:

(3 comments)

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@367
PS3, Line 367:   private static class RangerResourceResult {
             :     List<RangerResultRow> server = new ArrayList<>();
             :     List<RangerResultRow> uri = new ArrayList<>();
             :     List<RangerResultRow> database = new ArrayList<>();
             :     List<RangerResultRow> udf = new ArrayList<>();
             :     List<RangerResultRow> table = new ArrayList<>();
             :     List<RangerResultRow> column = new ArrayList<>();
make these private


http://gerrit.cloudera.org:8080/#/c/13673/3/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java@392
PS3, Line 392:     public RangerResourceResult addUdfResult(RangerResultRow 
result) {
             :       udf.add(result);
             :       return this;
             :     }
             :
             :     public RangerResourceResult addUriResult(RangerResultRow 
result) {
             :       uri.add(result);
             :       return this;
             :     }
These two methods are unused, that means uri and udf will always be empty.


http://gerrit.cloudera.org:8080/#/c/13673/3/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java@407
PS3, Line 407:     public List<RangerResultRow> getResultRows() {
             :       List<RangerResultRow> results = new ArrayList<>();
             :
             :       results.addAll(filterIfAll(server));
             :       results.addAll(filterIfAll(database));
             :       results.addAll(filterIfAll(table));
             :       results.addAll(filterIfAll(column));
             :       results.addAll(filterIfAll(udf));
             :       results.addAll(filterIfAll(uri));
             :
             :       return results;
             :     }
I don't quite follow the logic why we have to filter ALL. A comment will be 
helpful.



--
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: Fri, 28 Jun 2019 20:38:14 +0000
Gerrit-HasComments: Yes

Reply via email to