Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22591 )
Change subject: IMPALA-13841: Refactor AnalysisResult to make it immutable and simpler ...................................................................... Patch Set 6: (5 comments) http://gerrit.cloudera.org:8080/#/c/22591/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/22591/6/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@93 PS6, Line 93: private boolean userHasProfileAccess_ = true; Can we make this final as well? http://gerrit.cloudera.org:8080/#/c/22591/6/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@95 PS6, Line 95: public AnalysisResult(AnalysisDriver analysisDriver) { Lets not use AnalysisDriver as parameter. http://gerrit.cloudera.org:8080/#/c/22591/6/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@485 PS6, Line 485: authzChecker.postAnalyze(authzCtx); Can we make the authorization checking as part of AnalysisDriver as well? http://gerrit.cloudera.org:8080/#/c/22591/5/fe/src/main/java/org/apache/impala/analysis/AnalysisDriver.java File fe/src/main/java/org/apache/impala/analysis/AnalysisDriver.java: http://gerrit.cloudera.org:8080/#/c/22591/5/fe/src/main/java/org/apache/impala/analysis/AnalysisDriver.java@25 PS5, Line 25: public interface AnalysisDriver > I think so? I'm not familiar with what would be different. But this is mo Ack http://gerrit.cloudera.org:8080/#/c/22591/5/fe/src/main/java/org/apache/impala/analysis/AnalysisDriver.java@28 PS5, Line 28: public StatementBase getStmt(); : : public Analyzer getAnalyzer(); > Well, these aren't strictly necessary and I could get rid of them if you th Ack. I think I prefer latter AnalysisResult constructor. -- To view, visit http://gerrit.cloudera.org:8080/22591 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idaf8854b1ce18c72a49893fc36e2cea77b7a2485 Gerrit-Change-Number: 22591 Gerrit-PatchSet: 6 Gerrit-Owner: Steve Carlin <scar...@cloudera.com> Gerrit-Reviewer: Aman Sinha <amsi...@cloudera.com> Gerrit-Reviewer: Fang-Yu Rao <fangyu....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Michael Smith <michael.sm...@cloudera.com> Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Reviewer: Steve Carlin <scar...@cloudera.com> Gerrit-Comment-Date: Mon, 10 Mar 2025 17:37:50 +0000 Gerrit-HasComments: Yes
