You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "guojingfeng (Code Review)" <ge...@cloudera.org> on 2021/08/24 01:37:27 UTC

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

guojingfeng has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17781


Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................

IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs

This patch rollback some changes of IMPALA-9620 which initialize
SelectStmt's groupingExprs_ to ensure that group-by and cnf exprs are
analyzed. Since i noticed that IMPALA-9693 explicit analyze exprs
which is not analyzed yet. So this rollback is safe here.

Another reason why this patch is submitted is that re-initialize
SelectStmt's groupingExprs_ will cause other problems. IMPALA-10096 is
a typical case.

Here is a reproduce case that is similar to IMPALA-10096:
```
EXPLAIN
SELECT ss_item_sk ss_item_sk_group,
       ss_item_sk+300 ss_item_sk,
       count(ss_ticket_number)
FROM store_sales a
WHERE ss_sold_date_sk > cast('245263' AS INT)
GROUP BY ss_item_sk_group,
         ss_item_sk;
```
1. Expr cast('245263' AS INT) will hit expr rewrite logic
2. GroupBy exprs will substitude to SlotRef or FunctionCallExpr
3. ReAnalyze phase repeate step above

The problem is that if we change the groupingExprs_ of SelectStmt,
in re-analyze phase ss_item_sk alias ss_item_sk_group will be
substitude to Expr that duplicate with ss_item_sk which will be
removed in `buildAggregateExprs`. AnalyzeException
"select list expression not produced by aggregation output
(missing from GROUP BY clause?): ss_item_sk ss_item_sk_group" will
threw to client side.

This patch just remove the re-initialize groupingExprs_ of SelectStmt.

As a side effect, this patch modify ExtractCompundVerticalBarExprRule
to do a explicit analyze to ensure expr are rewritted.

Test:
- Add new test into aggregation.test and passed
- Ran all fe tests and passed

Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
---
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/rewrite/ExtractCompoundVerticalBarExprRule.java
M testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
3 files changed, 26 insertions(+), 15 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/17781/2
-- 
To view, visit http://gerrit.cloudera.org:8080/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 2
Gerrit-Owner: guojingfeng <gu...@tencent.com>

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

Posted by "guojingfeng (Code Review)" <ge...@cloudera.org>.
guojingfeng has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/17781 )

Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................

IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs

This patch rollback some changes of IMPALA-9620 which initialize
SelectStmt's groupingExprs_ to ensure that group-by and cnf exprs are
analyzed. Since i noticed that IMPALA-9693 explicit analyze exprs
which is not analyzed yet. So this rollback is safe here.

Another reason why this patch is submitted is that re-initialize
SelectStmt's groupingExprs_ will cause other problems. IMPALA-10096 is
a typical case.

Here is a reproduce case that is similar to IMPALA-10096:
```
EXPLAIN
SELECT ss_item_sk ss_item_sk_group,
       ss_item_sk+300 ss_item_sk,
       count(ss_ticket_number)
FROM store_sales a
WHERE ss_sold_date_sk > cast('245263' AS INT)
GROUP BY ss_item_sk_group,
         ss_item_sk;
```
1. Expr cast('245263' AS INT) will hit expr rewrite logic
2. GroupBy exprs will substitude to SlotRef or FunctionCallExpr
3. ReAnalyze phase repeate step above

The problem is that if we change the groupingExprs_ of SelectStmt,
in re-analyze phase ss_item_sk alias ss_item_sk_group will be
substitude to Expr that duplicate with ss_item_sk which will be
removed in `buildAggregateExprs`. AnalyzeException
"select list expression not produced by aggregation output
(missing from GROUP BY clause?): ss_item_sk ss_item_sk_group" will
threw to client side.

This patch just remove the re-initialize groupingExprs_ of SelectStmt.

As a side effect, this patch modify ExtractCompundVerticalBarExprRule
to do a explicit analyze to ensure expr are rewritted.

Test:
- Add new test into aggregation.test and passed
- Ran all fe tests and passed

Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
---
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/rewrite/ExtractCompoundVerticalBarExprRule.java
M testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
3 files changed, 28 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/17781/3
-- 
To view, visit http://gerrit.cloudera.org:8080/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 3
Gerrit-Owner: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/17781 )

Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................


Patch Set 5:

Failed because of IMPALA-10316, I am restarting the Verify job.


-- 
To view, visit http://gerrit.cloudera.org:8080/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 5
Gerrit-Owner: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Thu, 12 May 2022 05:33:33 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/17781 )

Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................


Patch Set 8: Code-Review+1


-- 
To view, visit http://gerrit.cloudera.org:8080/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 8
Gerrit-Owner: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Mon, 13 Jun 2022 08:29:48 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
wangsheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/17781 )

Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................


Patch Set 8: Code-Review+2

LGTM, thanks for modified.


-- 
To view, visit http://gerrit.cloudera.org:8080/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 8
Gerrit-Owner: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Mon, 13 Jun 2022 09:34:16 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

Posted by "guojingfeng (Code Review)" <ge...@cloudera.org>.
guojingfeng has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/17781 )

Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................

IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs

This patch rollback some changes of IMPALA-9620 which initialize
SelectStmt's groupingExprs_ to ensure that group-by and cnf exprs are
analyzed. Since i noticed that IMPALA-9693 explicit analyze exprs
which is not analyzed yet. So this rollback is safe here.

Another reason why this patch is submitted is that re-initialize
SelectStmt's groupingExprs_ will cause other problems. IMPALA-10096 is
a typical case.

Here is a reproduce case that is similar to IMPALA-10096:
```
EXPLAIN
SELECT ss_item_sk ss_item_sk_group,
       ss_item_sk+300 ss_item_sk,
       count(ss_ticket_number)
FROM store_sales a
WHERE ss_sold_date_sk > cast('245263' AS INT)
GROUP BY ss_item_sk_group,
         ss_item_sk;
```
1. Expr cast('245263' AS INT) will hit expr rewrite logic
2. GroupBy exprs will substitude to SlotRef or FunctionCallExpr
3. ReAnalyze phase repeate step above

The problem is that if we change the groupingExprs_ of SelectStmt,
in re-analyze phase ss_item_sk alias ss_item_sk_group will be
substitude to Expr that duplicate with ss_item_sk which will be
removed in `buildAggregateExprs`. AnalyzeException
"select list expression not produced by aggregation output
(missing from GROUP BY clause?): ss_item_sk ss_item_sk_group" will
threw to client side.

This patch just remove the re-initialize groupingExprs_ of SelectStmt.

As a side effect, this patch modify ExtractCompundVerticalBarExprRule
to do a explicit analyze to ensure expr are rewritted.

Test:
- Add new test into aggregation.test and passed
- Ran all fe tests and passed

Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
---
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/rewrite/ExtractCompoundVerticalBarExprRule.java
M testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
3 files changed, 28 insertions(+), 15 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/17781/4
-- 
To view, visit http://gerrit.cloudera.org:8080/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 4
Gerrit-Owner: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

Posted by "guojingfeng (Code Review)" <ge...@cloudera.org>.
guojingfeng has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/17781 )

Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................

IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs

This patch rollback some changes of IMPALA-9620. IMPALA-9620 re-
initialize SelectStmt's groupingExprs_ to ensure that group-by and
cnf exprs are analyzed. But the following IMPALA-9693 explicit analyze
exprs which is equivalent to IMPALA-9620. So this rollback is safe
here.

In general, the analyze algorithm is that:
1. Analyze the stmt tree and make copies of expressions
2. Rewrite selected expressions, **rewrite rules should keep rewritted
   exprs are analyzed**
3. Make copied expressions analyzed
4. ReAnalyze the tree

The problem is that if we change the groupingExprs_ of SelectStmt,
in re-analyze phase column alias will be substitude to Expr that
duplicate with origin column which will be removed in
`buildAggregateExprs`.

Another reason why this patch is submitted is that re-initialize
SelectStmt's groupingExprs_ will cause other problems. IMPALA-10096 is
a typical case. See jira for detail execeptions.

Beside, this patch modify ExtractCompundVerticalBarExprRule to do a
explicit analyze to ensure expr are rewritted.

Test:
- Add new test into aggregation.test and passed
- Ran all fe tests and passed

Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
---
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/rewrite/ExtractCompoundVerticalBarExprRule.java
M testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
3 files changed, 29 insertions(+), 15 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/17781/7
-- 
To view, visit http://gerrit.cloudera.org:8080/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 7
Gerrit-Owner: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17781 )

Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................


Patch Set 6:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10653/ : 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/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 6
Gerrit-Owner: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Sat, 28 May 2022 08:08:04 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17781 )

Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17781/2/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
File fe/src/main/java/org/apache/impala/analysis/SelectStmt.java:

http://gerrit.cloudera.org:8080/#/c/17781/2/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@381
PS2, Line 381:       if (LOG.isTraceEnabled()) LOG.trace("Analyzed select clause aliasSmap={}", aliasSmap_.debugString());
line too long (107 > 90)



-- 
To view, visit http://gerrit.cloudera.org:8080/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 2
Gerrit-Owner: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 24 Aug 2021 01:38:29 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17781 )

Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................


Patch Set 3:

Build Failed 

https://jenkins.impala.io/job/gerrit-code-review-checks/9352/ : Initial code review checks failed. See linked job for details on the failure.


-- 
To view, visit http://gerrit.cloudera.org:8080/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 3
Gerrit-Owner: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 24 Aug 2021 02:18:26 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17781 )

Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................


Patch Set 5:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/8095/ DRY_RUN=true


-- 
To view, visit http://gerrit.cloudera.org:8080/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 5
Gerrit-Owner: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Thu, 12 May 2022 05:34:18 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

Posted by "guojingfeng (Code Review)" <ge...@cloudera.org>.
guojingfeng has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/17781 )

Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................

IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs

This patch rollback some changes of IMPALA-9620. IMPALA-9620 re-
initialize SelectStmt's groupingExprs_ to ensure that group-by and
cnf exprs are analyzed. But the following patch of IMPALA-9693
explicitly analyzes exprs which is equivalent to IMPALA-9620. So this
rollback is safe here.

In general, the analyze algorithm is that:
1. Analyze the stmt tree and make copies of expressions
2. Rewrite selected expressions, **rewrite rules should ensure
   rewritten exprs are analyzed**
3. Make copied expressions analyzed
4. ReAnalyze the tree

The problem is that if we change the groupingExprs_ of SelectStmt,
in re-analyze phase column alias will be substitude to Expr that
duplicate with origin column which will be removed in
`buildAggregateExprs`.

Another reason why this patch is submitted is that re-initialize
SelectStmt's groupingExprs_ will cause other problems. IMPALA-10096 is
a typical case. See jira for detail execeptions.

Beside, this patch modifies ExtractCompundVerticalBarExprRule to do a
explicit analyze to ensure expr are rewritten.

Test:
- Add new test into aggregation.test and passed
- Ran all fe tests and passed

Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
---
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/rewrite/ExtractCompoundVerticalBarExprRule.java
M testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
3 files changed, 29 insertions(+), 15 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/17781/8
-- 
To view, visit http://gerrit.cloudera.org:8080/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 8
Gerrit-Owner: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17781 )

Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................


Patch Set 10: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 10
Gerrit-Owner: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Fri, 24 Jun 2022 07:41:41 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
wangsheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/17781 )

Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................


Patch Set 4:

(5 comments)

Hi guojing, thanks for this bugfix!

http://gerrit.cloudera.org:8080/#/c/17781/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17781/4//COMMIT_MSG@9
PS4, Line 9: This patch rollback some changes of IMPALA-9620 which initialize
This patch also rollback some changes in IMPALA-10096, so maybe we can also explain this at first. Besides, I think this patch description maybe complex, can you reorganize the language to make it clearer and easier to understand?


http://gerrit.cloudera.org:8080/#/c/17781/4/fe/src/main/java/org/apache/impala/rewrite/ExtractCompoundVerticalBarExprRule.java
File fe/src/main/java/org/apache/impala/rewrite/ExtractCompoundVerticalBarExprRule.java:

http://gerrit.cloudera.org:8080/#/c/17781/4/fe/src/main/java/org/apache/impala/rewrite/ExtractCompoundVerticalBarExprRule.java@38
PS4, Line 38:       if (!expr.isAnalyzed()) {
            :         pred = (CompoundVerticalBarExpr) expr.clone();
            :         pred.analyzeNoThrow(analyzer);
            :       }
This change seems unnecessary to this patch. Without this change, I can also execute this query successful:  
SELECT ss_item_sk ss_item_sk_group,
       ss_item_sk+300 ss_item_sk,
       count(ss_ticket_number)
FROM store_sales a
WHERE ss_sold_date_sk > cast('245263' AS INT)
GROUP BY ss_item_sk_group,
         ss_item_sk;


http://gerrit.cloudera.org:8080/#/c/17781/4/testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
File testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test:

http://gerrit.cloudera.org:8080/#/c/17781/4/testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test@1668
PS4, Line 1668: 3
Maybe we should change this in another commit?


http://gerrit.cloudera.org:8080/#/c/17781/4/testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test@1675
PS4, Line 1675: # Group by expr with column alias reference errors in re-analyze phase
Maybe we can add Jira here.


http://gerrit.cloudera.org:8080/#/c/17781/4/testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test@1676
PS4, Line 1676:  
unnecessary



-- 
To view, visit http://gerrit.cloudera.org:8080/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 4
Gerrit-Owner: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Tue, 07 Sep 2021 07:09:45 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/17781 )

Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................


Patch Set 8: Code-Review+2

Hi Guojing, thank you for this fix, LGTM, starting the verify job.


-- 
To view, visit http://gerrit.cloudera.org:8080/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 8
Gerrit-Owner: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Wed, 22 Jun 2022 08:02:18 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17781 )

Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................


Patch Set 9: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 9
Gerrit-Owner: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Wed, 22 Jun 2022 08:02:54 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17781 )

Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................

IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs

This patch rollback some changes of IMPALA-9620. IMPALA-9620 re-
initialize SelectStmt's groupingExprs_ to ensure that group-by and
cnf exprs are analyzed. But the following patch of IMPALA-9693
explicitly analyzes exprs which is equivalent to IMPALA-9620. So this
rollback is safe here.

In general, the analyze algorithm is that:
1. Analyze the stmt tree and make copies of expressions
2. Rewrite selected expressions, **rewrite rules should ensure
   rewritten exprs are analyzed**
3. Make copied expressions analyzed
4. ReAnalyze the tree

The problem is that if we change the groupingExprs_ of SelectStmt,
in re-analyze phase column alias will be substitude to Expr that
duplicate with origin column which will be removed in
`buildAggregateExprs`.

Another reason why this patch is submitted is that re-initialize
SelectStmt's groupingExprs_ will cause other problems. IMPALA-10096 is
a typical case. See jira for detail execeptions.

Beside, this patch modifies ExtractCompundVerticalBarExprRule to do a
explicit analyze to ensure expr are rewritten.

Test:
- Add new test into aggregation.test and passed
- Ran all fe tests and passed

Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Reviewed-on: http://gerrit.cloudera.org:8080/17781
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/rewrite/ExtractCompoundVerticalBarExprRule.java
M testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
3 files changed, 29 insertions(+), 15 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 11
Gerrit-Owner: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/17781 )

Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................


Patch Set 9: Code-Review+2

The failure looks like IMPALA-10927, restarting the verify job.


-- 
To view, visit http://gerrit.cloudera.org:8080/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 9
Gerrit-Owner: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Fri, 24 Jun 2022 07:41:08 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17781 )

Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................


Patch Set 10:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/8267/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 10
Gerrit-Owner: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Fri, 24 Jun 2022 07:41:42 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/17781 )

Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................


Patch Set 7: Code-Review+1

(5 comments)

http://gerrit.cloudera.org:8080/#/c/17781/7//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17781/7//COMMIT_MSG@11
PS7, Line 11: But the following IMPALA-9693 explicit analyze
nit: But the following patch of IMPALA-9693 explicitly analyzes


http://gerrit.cloudera.org:8080/#/c/17781/7//COMMIT_MSG@17
PS7, Line 17: rewrite rules should keep rewritted
            :    exprs are analyzed
nit: reword to "rewrite rules should ensure rewritten exprs are analyzed"


http://gerrit.cloudera.org:8080/#/c/17781/7//COMMIT_MSG@31
PS7, Line 31: modify
nit: modifies


http://gerrit.cloudera.org:8080/#/c/17781/7//COMMIT_MSG@32
PS7, Line 32: rewritted
nit: rewritted -> rewritten


http://gerrit.cloudera.org:8080/#/c/17781/7/testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
File testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test:

http://gerrit.cloudera.org:8080/#/c/17781/7/testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test@1677
PS7, Line 1677: explain
We don't need EXPLAIN here. The queries in PlannerTest are not actually executed. We just get the exec request and verify the plans:
https://github.com/apache/impala/blob/9baf790606073d88c3a2fd431110812140df0cb7/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java#L519-L520



-- 
To view, visit http://gerrit.cloudera.org:8080/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 7
Gerrit-Owner: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Mon, 06 Jun 2022 08:11:44 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17781 )

Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................


Patch Set 5:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/8091/ DRY_RUN=true


-- 
To view, visit http://gerrit.cloudera.org:8080/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 5
Gerrit-Owner: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Wed, 11 May 2022 09:34:38 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17781 )

Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................


Patch Set 5: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/8091/


-- 
To view, visit http://gerrit.cloudera.org:8080/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 5
Gerrit-Owner: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Wed, 11 May 2022 14:07:48 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

Posted by "guojingfeng (Code Review)" <ge...@cloudera.org>.
guojingfeng has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/17781 )

Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................

IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs

This patch rollback some changes of IMPALA-9620. IMPALA-9620 re-
initialize SelectStmt's groupingExprs_ to ensure that group-by and
cnf exprs are analyzed. But the following IMPALA-9693 explicit analyze
exprs which is equivalent to IMPALA-9620. So this rollback is safe
here.

In general, the analyze algorithm is that:
1. Analyze the stmt tree and make copies of expressions
2. Rewrite selected expressions, **rewrite rules should keep rewritted
   exprs are analyzed**
3. Make copied expressions analyzed
4. ReAnalyze the tree

The problem is that if we change the groupingExprs_ of SelectStmt,
in re-analyze phase column alias will be substitude to Expr that
duplicate with origin column which will be removed in
`buildAggregateExprs`.

Another reason why this patch is submitted is that re-initialize
SelectStmt's groupingExprs_ will cause other problems. IMPALA-10096 is
a typical case. See jira for detail execeptions.

Beside, this patch modify ExtractCompundVerticalBarExprRule to do a
explicit analyze to ensure expr are rewritted.

Test:
- Add new test into aggregation.test and passed
- Ran all fe tests and passed

Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
---
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/rewrite/ExtractCompoundVerticalBarExprRule.java
M testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
3 files changed, 28 insertions(+), 15 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/17781/6
-- 
To view, visit http://gerrit.cloudera.org:8080/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 6
Gerrit-Owner: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17781 )

Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................


Patch Set 9:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/8258/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 9
Gerrit-Owner: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Wed, 22 Jun 2022 08:02:55 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

Posted by "guojingfeng (Code Review)" <ge...@cloudera.org>.
guojingfeng has posted comments on this change. ( http://gerrit.cloudera.org:8080/17781 )

Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................


Patch Set 8:

(1 comment)

Thanks for your suggestions Quanlong, i upload new patch according your comments.

> Patch Set 7: Code-Review+1
> 
> (5 comments)

http://gerrit.cloudera.org:8080/#/c/17781/7/testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
File testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test:

http://gerrit.cloudera.org:8080/#/c/17781/7/testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test@1677
PS7, Line 1677: SELECT 
> We don't need EXPLAIN here. The queries in PlannerTest are not actually exe
Thanks for explain, i remove it.



-- 
To view, visit http://gerrit.cloudera.org:8080/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 8
Gerrit-Owner: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Wed, 08 Jun 2022 14:57:25 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17781 )

Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9351/ : 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/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 2
Gerrit-Owner: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 24 Aug 2021 02:00:46 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17781 )

Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/9373/ : 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/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 4
Gerrit-Owner: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Aug 2021 07:02:47 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17781 )

Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................


Patch Set 5: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 5
Gerrit-Owner: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Thu, 12 May 2022 09:59:49 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17781 )

Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................


Patch Set 8:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10733/ : 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/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 8
Gerrit-Owner: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Wed, 08 Jun 2022 15:17:15 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17781 )

Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................


Patch Set 9: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/8258/


-- 
To view, visit http://gerrit.cloudera.org:8080/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 9
Gerrit-Owner: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Wed, 22 Jun 2022 12:45:10 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17781 )

Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................


Patch Set 10: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 10
Gerrit-Owner: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Fri, 24 Jun 2022 12:20:16 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17781 )

Change subject: IMPALA-10865: Fix initialize SelectStmt's groupingExprs_ in analyzeGroupingExprs
......................................................................


Patch Set 7:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/10654/ : 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/17781
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d1779e6c282d9fd02beacf5ddfafcc5c0baf3b0
Gerrit-Change-Number: 17781
Gerrit-PatchSet: 7
Gerrit-Owner: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: guojingfeng <gu...@tencent.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Sat, 28 May 2022 08:22:04 +0000
Gerrit-HasComments: No