[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: guojingfeng Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Fri, 24 Jun 2022 12:20:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs
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 Tested-by: Impala Public Jenkins --- 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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: guojingfeng Gerrit-Reviewer: wangsheng
[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: guojingfeng Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Fri, 24 Jun 2022 07:41:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: guojingfeng Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Fri, 24 Jun 2022 07:41:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: guojingfeng Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Fri, 24 Jun 2022 07:41:41 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: guojingfeng Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Wed, 22 Jun 2022 12:45:10 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: guojingfeng Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Wed, 22 Jun 2022 08:02:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: guojingfeng Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Wed, 22 Jun 2022 08:02:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: guojingfeng Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Wed, 22 Jun 2022 08:02:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: guojingfeng Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Mon, 13 Jun 2022 09:34:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: guojingfeng Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Mon, 13 Jun 2022 08:29:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: guojingfeng Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Wed, 08 Jun 2022 15:17:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: guojingfeng Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Wed, 08 Jun 2022 14:57:25 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: guojingfeng Gerrit-Reviewer: wangsheng
[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: guojingfeng Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Mon, 06 Jun 2022 08:11:44 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: guojingfeng Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Sat, 28 May 2022 08:22:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: guojingfeng Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Sat, 28 May 2022 08:08:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: guojingfeng Gerrit-Reviewer: wangsheng
[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: wangsheng
[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Thu, 12 May 2022 09:59:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Thu, 12 May 2022 05:33:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tamas Mate Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Thu, 12 May 2022 05:34:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Wed, 11 May 2022 14:07:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Wed, 11 May 2022 09:34:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs
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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Tue, 07 Sep 2021 07:09:45 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs
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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 26 Aug 2021 07:02:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs
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 Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs
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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 24 Aug 2021 02:18:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs
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 Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs
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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 24 Aug 2021 02:00:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10865: Fix initialize SelectStmt's groupingExprs in analyzeGroupingExprs
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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 24 Aug 2021 01:38:29 + Gerrit-HasComments: Yes
[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