[Impala-ASF-CR] IMPALA-10619: Minor refactoring of standardize method for analytic functions
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
Gerrit-Reviewer: Aman Sinha
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Comment-Date: Fri, 02 Apr 2021 13:16:11 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10619: Minor refactoring of standardize method for analytic functions
Aman Sinha 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: (1 comment) 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@833 PS1, Line 833: protected FunctionCallExpr createRewrittenFunction(FunctionName funcName, > This seems a static method for FunctionCallExpr. Nothing special for analyt Right..this was a natural and object-oriented way I could think of where a derived class of AnalyticExpr could override it. The external FE would create something like a new ExternalFEFunctionCallExpr (this is just an example name) which is a derived class of FunctionCallExpr. Making it a static method would prevent the override. I am open to suggestions and will try to think of alternatives. -- 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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Wed, 31 Mar 2021 06:44:24 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10619: Minor refactoring of standardize method for analytic functions
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: (1 comment) 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@833 PS1, Line 833: protected FunctionCallExpr createRewrittenFunction(FunctionName funcName, This seems a static method for FunctionCallExpr. Nothing special for analytic. Are we putting it here just for subclasses to override it? Is it possible to move it to FunctionCallExpr? -- 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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Tue, 30 Mar 2021 12:51:38 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10619: Minor refactoring of standardize method for analytic functions
Impala Public Jenkins 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: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/8461/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 30 Mar 2021 00:46:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10619: Minor refactoring of standardize method for analytic functions
Aman Sinha has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17237 Change subject: IMPALA-10619: Minor refactoring of standardize method for analytic functions .. IMPALA-10619: Minor refactoring of standardize method for analytic functions The FIRST_VALUE, LAST_VALUE functions go through standardization process in AnalyticExpr where they may be rewritten with different number of parameters or with different window frame. In order for an external FE to leverage this standardization, this patch creates a wrapper method for FunctionCallExpr creation and does minor refactoring. Testing: Ran PlannerTests. No new tests are added since this does not change the existing behavior. Change-Id: I39e4268c0c5500f09acf98357a80763c28f615c2 --- M fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java 1 file changed, 12 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/17237/1 -- 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: newchange Gerrit-Change-Id: I39e4268c0c5500f09acf98357a80763c28f615c2 Gerrit-Change-Number: 17237 Gerrit-PatchSet: 1 Gerrit-Owner: Aman Sinha
