Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/17237 )
Change subject: IMPALA-10619: Minor refactoring of standardize method for analytic functions ...................................................................... Patch Set 1: (4 comments) 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? http://gerrit.cloudera.org:8080/#/c/17237/1/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java@335 PS1, Line 335: new FunctionCallExpr("if", ifParams) same question as above http://gerrit.cloudera.org:8080/#/c/17237/1/fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java@353 PS1, Line 353: new FunctionCallExpr same question as above 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, > Right..this was a natural and object-oriented way I could think of where a I'm ok if we don't have a better way. Just concerns that whether future changes will break the external frontend if they use the FunctionCallExpr constructor directly. As other comments in this file, there are several other places that we still use the constructor. Maybe we need to wrap them as well. -- 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: 1 Gerrit-Owner: Aman Sinha <amsi...@cloudera.com> Gerrit-Reviewer: Aman Sinha <amsi...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Comment-Date: Fri, 02 Apr 2021 13:16:11 +0000 Gerrit-HasComments: Yes