Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/17078 )
Change subject: IMPALA-10505: Avoid creating misleading audit logs ...................................................................... Patch Set 2: (4 comments) The fix looks good to me. Just have few minor comments. http://gerrit.cloudera.org:8080/#/c/17078/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17078/2//COMMIT_MSG@47 PS2, Line 47: Verified that a user not granted the privilege on the underlying : table(s) of a view is still not able to access the runtime profile or : execution summary even though the user is granted the privilege on : the view. Do we have test for this? http://gerrit.cloudera.org:8080/#/c/17078/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java: http://gerrit.cloudera.org:8080/#/c/17078/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@473 PS2, Line 473: tmpAuditHandler.getAuthzEvents().clear(); I have another thought that if we don't need these audits, we don't need to generate them at all. What about setting tmpAuditHandler to null when retainAudits=false? Then we don't need this if-clause and just need to add check of 'tmpAuditHandler != null' at line 475. http://gerrit.cloudera.org:8080/#/c/17078/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@505 PS2, Line 505: tmpAuditHandler.getAuthzEvents().clear(); Same point as the above comment. http://gerrit.cloudera.org:8080/#/c/17078/2/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java File fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java: http://gerrit.cloudera.org:8080/#/c/17078/2/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java@163 PS2, Line 163: nit: two blank lines here, please remove one. -- To view, visit http://gerrit.cloudera.org:8080/17078 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I02f40eb96d6ed863cd2cd88d717c354dc351a64c Gerrit-Change-Number: 17078 Gerrit-PatchSet: 2 Gerrit-Owner: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Mon, 22 Feb 2021 09:11:58 +0000 Gerrit-HasComments: Yes
