Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/13285 )
Change subject: IMPALA-8528: Refactor authorization check in AnalysisContext ...................................................................... Patch Set 6: (2 comments) Not quite following the pre/post/context stuff. I'm guessing this will be used for audit? Probably cleaner to just keep this to a refactor (moving code around) and not also introduce this new "context" concept until it's needed for audits? http://gerrit.cloudera.org:8080/#/c/13285/6/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java File fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java: http://gerrit.cloudera.org:8080/#/c/13285/6/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@423 PS6, Line 423: authzCtx = authzChecker.preAuthorize(analysisResult_, catalog_); I'm not entirely following the purpose of the pre/post stuff here. Couldn't this all be done within the 'authorize()' call itself? http://gerrit.cloudera.org:8080/#/c/13285/6/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java File fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java: http://gerrit.cloudera.org:8080/#/c/13285/6/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java@38 PS6, Line 38: * Executes some code before the authorization check. this seems to be a bit more in that it's also supposed to return an authorization context -- To view, visit http://gerrit.cloudera.org:8080/13285 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3bc3a11220dae0f49ef3e73d9ff27a90e9d4a71c Gerrit-Change-Number: 13285 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Austin Nobis <[email protected]> Gerrit-Reviewer: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Mon, 13 May 2019 21:58:05 +0000 Gerrit-HasComments: Yes
