Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/17237 )
Change subject: IMPALA-10619: Minor refactoring of analytic function methods ...................................................................... Patch Set 2: (2 comments) Responses below. Also, in PS 2, I added 3 accessor methods and changed modifiers which were needed for analytic functions. http://gerrit.cloudera.org:8080/#/c/17237/1/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java File fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java: http://gerrit.cloudera.org:8080/#/c/17237/1/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java@280 PS1, Line 280: new FunctionCallExpr("if", ifParams) > Should this be wrapped as well? Pls see related response below. http://gerrit.cloudera.org:8080/#/c/17237/1/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java@833 PS1, Line 833: protected FunctionCallExpr createRewrittenFunction(FunctionName funcName, > I'm ok if we don't have a better way. Just concerns that whether future cha Sorry, was distracted by some other issues. I did look into wrapping those 3 invocations you mentioned. The problem is those are static methods whereas the createRewrittenFunction() needs to be an instance method. Your point about future changes causing some breakage in the external frontend is valid. In the short term, the expectation is that we will run the tests ourselves to detect potential issues on a regular basis but in the medium term we do want to address it in a general manner such that code compilation itself would catch the issue instead of testing. -- To view, visit http://gerrit.cloudera.org:8080/17237 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I39e4268c0c5500f09acf98357a80763c28f615c2 Gerrit-Change-Number: 17237 Gerrit-PatchSet: 2 Gerrit-Owner: Aman Sinha <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Wed, 07 Apr 2021 22:40:31 +0000 Gerrit-HasComments: Yes
