You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Aman Sinha (Code Review)" <ge...@cloudera.org> on 2021/03/30 00:26:04 UTC
[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 <am...@cloudera.com>
[Impala-ASF-CR] IMPALA-10619: Minor refactoring of standardize method for analytic functions
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
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 <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 31 Mar 2021 06:44:24 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10619: Minor refactoring of analytic function methods
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17237 )
Change subject: IMPALA-10619: Minor refactoring of analytic function methods
......................................................................
Patch Set 4: Code-Review+2
--
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: 4
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 12 Apr 2021 07:22:05 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10619: Minor refactoring of standardize method for analytic functions
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
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 <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 02 Apr 2021 13:16:11 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10619: Minor refactoring of analytic function methods
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17237 )
Change subject: IMPALA-10619: Minor refactoring of analytic function methods
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/8520/ : 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: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 07 Apr 2021 23:03:32 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10619: Minor refactoring of standardize method for analytic functions
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
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 <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 30 Mar 2021 12:51:38 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10619: Minor refactoring of analytic function methods
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
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 4: Code-Review+2
Carry forward +2
--
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: 4
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 12 Apr 2021 21:06:35 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10619: Minor refactoring of analytic function methods
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17237 )
Change subject: IMPALA-10619: Minor refactoring of analytic function methods
......................................................................
Patch Set 4: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/7063/
--
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: 4
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 12 Apr 2021 13:12:12 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10619: Minor refactoring of analytic function methods
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
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 4: Verified+1
> Patch Set 4: Verified-1
>
> Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/7063/
2 HBASE test failures are flaky tests as mentioned in IMPALA-1995.
--
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: 4
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 12 Apr 2021 17:51:56 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10619: Minor refactoring of analytic function methods
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/17237 )
Change subject: IMPALA-10619: Minor refactoring of analytic function methods
......................................................................
Patch Set 3: Code-Review+2
(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,
> Sorry, was distracted by some other issues. I did look into wrapping those
Sure. Don't want to block this. I'm ok with this since I think the external frontend can override the standardize() method and add checks at the end to make sure the fnCall_ is in the desired class (e.g. ExternalFEFunctionCallExpr).
--
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: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 12 Apr 2021 00:18:17 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10619: Minor refactoring of analytic function methods
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
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 <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 07 Apr 2021 22:40:31 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10619: Minor refactoring of analytic function methods
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17237
to look at the new patch set (#3).
Change subject: IMPALA-10619: Minor refactoring of analytic function methods
......................................................................
IMPALA-10619: Minor refactoring of analytic function methods
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.
Also added accessor methods to AnalyticEvalNode and changed visibility
of couple of methods in PlanNode for use by external FE.
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
M fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java
M fe/src/main/java/org/apache/impala/planner/PlanNode.java
3 files changed, 18 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/17237/3
--
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: newpatchset
Gerrit-Change-Id: I39e4268c0c5500f09acf98357a80763c28f615c2
Gerrit-Change-Number: 17237
Gerrit-PatchSet: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-10619: Minor refactoring of analytic function methods
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17237 )
Change subject: IMPALA-10619: Minor refactoring of analytic function methods
......................................................................
IMPALA-10619: Minor refactoring of analytic function methods
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.
Also added accessor methods to AnalyticEvalNode and changed visibility
of couple of methods in PlanNode for use by external FE.
Testing: Ran PlannerTests. No new tests are added since this does not
change the existing behavior.
Change-Id: I39e4268c0c5500f09acf98357a80763c28f615c2
Reviewed-on: http://gerrit.cloudera.org:8080/17237
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Aman Sinha <am...@cloudera.com>
Reviewed-by: Aman Sinha <am...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
M fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java
M fe/src/main/java/org/apache/impala/planner/PlanNode.java
3 files changed, 18 insertions(+), 7 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved
Aman Sinha: Looks good to me, approved; Verified
--
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: merged
Gerrit-Change-Id: I39e4268c0c5500f09acf98357a80763c28f615c2
Gerrit-Change-Number: 17237
Gerrit-PatchSet: 5
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-10619: Minor refactoring of analytic function methods
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17237
to look at the new patch set (#2).
Change subject: IMPALA-10619: Minor refactoring of analytic function methods
......................................................................
IMPALA-10619: Minor refactoring of analytic function methods
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.
Also added accessor methods to AnalyticEvalNode and changed visibility
of couple of methods in PlanNode for use by external FE.
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
M fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java
M fe/src/main/java/org/apache/impala/planner/PlanNode.java
3 files changed, 18 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/17237/2
--
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: newpatchset
Gerrit-Change-Id: I39e4268c0c5500f09acf98357a80763c28f615c2
Gerrit-Change-Number: 17237
Gerrit-PatchSet: 2
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-10619: Minor refactoring of analytic function methods
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
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 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17237/2/fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java
File fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java:
http://gerrit.cloudera.org:8080/#/c/17237/2/fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java@673
PS2, Line 673:
> line has trailing whitespace
Done
--
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: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 07 Apr 2021 22:42:55 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10619: Minor refactoring of analytic function methods
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/8519/ : 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: 2
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 07 Apr 2021 22:33:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10619: Minor refactoring of analytic function methods
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17237/2/fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java
File fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java:
http://gerrit.cloudera.org:8080/#/c/17237/2/fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java@673
PS2, Line 673:
line has trailing whitespace
--
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 <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 07 Apr 2021 22:14:55 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10619: Minor refactoring of analytic function methods
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
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 3:
(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,
> Sure. Don't want to block this. I'm ok with this since I think the external
Yes, that's indeed how it is being checked in the derived class ..through a Preconditions.checkArgument on the instance of getFnCall().
--
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: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 12 Apr 2021 07:20:49 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10619: Minor refactoring of standardize method for analytic functions
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 30 Mar 2021 00:46:34 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10619: Minor refactoring of analytic function methods
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17237 )
Change subject: IMPALA-10619: Minor refactoring of analytic function methods
......................................................................
Patch Set 4:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7063/ DRY_RUN=false
--
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: 4
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 12 Apr 2021 07:22:06 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10619: Minor refactoring of analytic function methods
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has removed a vote on this change.
Change subject: IMPALA-10619: Minor refactoring of analytic function methods
......................................................................
Removed Verified-1 by Impala Public Jenkins <im...@cloudera.com>
--
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: deleteVote
Gerrit-Change-Id: I39e4268c0c5500f09acf98357a80763c28f615c2
Gerrit-Change-Number: 17237
Gerrit-PatchSet: 4
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>