Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/15854 )
Change subject: IMPALA-9597: Eliminate redundant Ranger audits for column masking ...................................................................... Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/15854/3/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java: http://gerrit.cloudera.org:8080/#/c/15854/3/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java@40 PS3, Line 40: private List<AuthzAuditEvent> stashedAuditEvents_; > Thanks Quanlong! If we use a Map<String, AuthzAuditEvent> to stash the even Looks like we never use stashedAuditEvents_ if the authorization failed. Is it just for performance reason to keep the stashedAuditEvent list? But appending to the list is nearly the same perf as inserting to the hashmap... http://gerrit.cloudera.org:8080/#/c/15854/5/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java: http://gerrit.cloudera.org:8080/#/c/15854/5/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java@48 PS5, Line 48: HashMap I think we should use LinkedHashMap to keep the insert order. -- To view, visit http://gerrit.cloudera.org:8080/15854 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I42d60130fba93d63fbc36949f2bf746b7ae2497d Gerrit-Change-Number: 15854 Gerrit-PatchSet: 5 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: Tue, 12 May 2020 01:16:32 +0000 Gerrit-HasComments: Yes
