You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "wangsheng (Code Review)" <ge...@cloudera.org> on 2021/12/01 12:02:50 UTC
[Impala-ASF-CR] IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
wangsheng has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18061
Change subject: IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
......................................................................
IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
When query contains multiple nested union stmt, more than twenty or
thirty, and needs 'reAnalyze', such as rewrite expr. Query would
execute slowly, even failed due to 'reset' method called in class
'UnionStmt' and 'SetOperationStmt'.
'SetOperationStmt' is added in IMPALA-9943 and IMPALA-4974. Multiple
nested union stmt will lead to 'reset' called numbers to grow
exponentially. Since 'operands_' will be reset in two class' reset()
method, and handle with their children recursively. Too many nested
union stmt will caused depth nesting.
UnionStmt.reset() content is almost same as SetOperationStmt.reset().
This patch removed this method in 'UnionStmt'. After this, the
original query would execute quickly.
An example already add in file 'union.test', without this patch, the
example query will execute slowly, maybe be failed.
Testing:
- Added new test case in 'union.test'
Change-Id: I408a396d40d9622f2ae6c459f49cbfcc19affd14
---
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M testdata/workloads/functional-query/queries/QueryTest/union.test
2 files changed, 129 insertions(+), 20 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/18061/1
--
To view, visit http://gerrit.cloudera.org:8080/18061
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I408a396d40d9622f2ae6c459f49cbfcc19affd14
Gerrit-Change-Number: 18061
Gerrit-PatchSet: 1
Gerrit-Owner: wangsheng <sk...@163.com>
[Impala-ASF-CR] IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
wangsheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/18061 )
Change subject: IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
......................................................................
Patch Set 1:
Hi, I found most of the members been clear in UnionStmt.reset() is 'SetOperand' type, and this class not been extends by other class. So I think it unnecessay to reset these members in both UnionStmt.reset() and SetOperationStmt.reset().
--
To view, visit http://gerrit.cloudera.org:8080/18061
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I408a396d40d9622f2ae6c459f49cbfcc19affd14
Gerrit-Change-Number: 18061
Gerrit-PatchSet: 1
Gerrit-Owner: wangsheng <sk...@163.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: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Wed, 01 Dec 2021 12:06:32 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Aman Sinha, Qifan Chen, Zoltan Borok-Nagy, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18061
to look at the new patch set (#4).
Change subject: IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
......................................................................
IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
When query contains multiple nested union stmt, more than twenty or
thirty, and needs 'reAnalyze', such as rewrite expr. Query would
execute slowly, even failed due to 'reset' method called in class
'UnionStmt' and 'SetOperationStmt'.
'SetOperationStmt' is added in IMPALA-9943 and IMPALA-4974. Multiple
nested union stmt will lead to 'reset' called numbers to grow
exponentially. Since 'operands_' will be reset in two class' reset()
method, and handle with their children recursively. Too many nested
union stmt will cause deep nesting.
UnionStmt.reset() content is exactly same as SetOperationStmt.reset().
This patch removed this method in 'UnionStmt'. After this, the
original query would execute quickly.
An example already add in file 'union.test', without this patch, the
example query would execute slowly, or even fail.
Testing:
- Added new test case in 'union.test'
Change-Id: I408a396d40d9622f2ae6c459f49cbfcc19affd14
---
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M testdata/workloads/functional-query/queries/QueryTest/union.test
2 files changed, 129 insertions(+), 20 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/18061/4
--
To view, visit http://gerrit.cloudera.org:8080/18061
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I408a396d40d9622f2ae6c459f49cbfcc19affd14
Gerrit-Change-Number: 18061
Gerrit-PatchSet: 4
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
[Impala-ASF-CR] IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/18061 )
Change subject: IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
......................................................................
Patch Set 3: Code-Review+2
(1 comment)
Thanks again for fixing this.
http://gerrit.cloudera.org:8080/#/c/18061/2/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
File fe/src/main/java/org/apache/impala/analysis/UnionStmt.java:
http://gerrit.cloudera.org:8080/#/c/18061/2/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java@a67
PS2, Line 67:
> Hi Zoltan, thanks for your patient review, but I don't understand. Do you m
Sorry, you're right, we can delete this method.
--
To view, visit http://gerrit.cloudera.org:8080/18061
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I408a396d40d9622f2ae6c459f49cbfcc19affd14
Gerrit-Change-Number: 18061
Gerrit-PatchSet: 3
Gerrit-Owner: wangsheng <sk...@163.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, 08 Dec 2021 10:32:15 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
Posted by "Qifan Chen (Code Review)" <ge...@cloudera.org>.
Qifan Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/18061 )
Change subject: IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
......................................................................
Patch Set 5: Code-Review+1
Thanks for the update!
--
To view, visit http://gerrit.cloudera.org:8080/18061
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I408a396d40d9622f2ae6c459f49cbfcc19affd14
Gerrit-Change-Number: 18061
Gerrit-PatchSet: 5
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@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: Thu, 09 Dec 2021 15:49:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/18061 )
Change subject: IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
......................................................................
Patch Set 2:
(4 comments)
Thanks for working on this fix.
http://gerrit.cloudera.org:8080/#/c/18061/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18061/2//COMMIT_MSG@17
PS2, Line 17: caused depth nesting.
nit: cause deep nesting.
http://gerrit.cloudera.org:8080/#/c/18061/2//COMMIT_MSG@22
PS2, Line 22: will execute slowly, maybe be failed.
nit: would execute slowly, or even fail.
http://gerrit.cloudera.org:8080/#/c/18061/2/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
File fe/src/main/java/org/apache/impala/analysis/UnionStmt.java:
http://gerrit.cloudera.org:8080/#/c/18061/2/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java@a67
PS2, Line 67:
I think we probably want this to be executed, we just don't want it to be executed redundantly. Can we just add a flag, e.g. 'boolean resetWasInvoked_;' and return from this function early when it's true? And set it to false in 'analyze()'?
http://gerrit.cloudera.org:8080/#/c/18061/2/testdata/workloads/functional-query/queries/QueryTest/union.test
File testdata/workloads/functional-query/queries/QueryTest/union.test:
http://gerrit.cloudera.org:8080/#/c/18061/2/testdata/workloads/functional-query/queries/QueryTest/union.test@1187
PS2, Line 1187: will execute slowly, even failed.
nit: would execute slowly, or even fail.
--
To view, visit http://gerrit.cloudera.org:8080/18061
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I408a396d40d9622f2ae6c459f49cbfcc19affd14
Gerrit-Change-Number: 18061
Gerrit-PatchSet: 2
Gerrit-Owner: wangsheng <sk...@163.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: Tue, 07 Dec 2021 17:03:17 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18061 )
Change subject: IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/18061
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I408a396d40d9622f2ae6c459f49cbfcc19affd14
Gerrit-Change-Number: 18061
Gerrit-PatchSet: 5
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@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: Thu, 09 Dec 2021 23:17:19 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
Posted by "Qifan Chen (Code Review)" <ge...@cloudera.org>.
Qifan Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/18061 )
Change subject: IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
......................................................................
Patch Set 3:
(1 comment)
Looks good to me too!
http://gerrit.cloudera.org:8080/#/c/18061/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18061/3//COMMIT_MSG@18
PS3, Line 18: almost same as
nit. exactly the same (since it calls super.reset() first).
--
To view, visit http://gerrit.cloudera.org:8080/18061
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I408a396d40d9622f2ae6c459f49cbfcc19affd14
Gerrit-Change-Number: 18061
Gerrit-PatchSet: 3
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@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, 08 Dec 2021 16:55:27 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
wangsheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/18061 )
Change subject: IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
......................................................................
Patch Set 4:
(1 comment)
Thanks for review!
http://gerrit.cloudera.org:8080/#/c/18061/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18061/3//COMMIT_MSG@18
PS3, Line 18: exactly same a
> nit. exactly the same (since it calls super.reset() first).
Done
--
To view, visit http://gerrit.cloudera.org:8080/18061
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I408a396d40d9622f2ae6c459f49cbfcc19affd14
Gerrit-Change-Number: 18061
Gerrit-PatchSet: 4
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@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: Thu, 09 Dec 2021 06:16:45 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18061 )
Change subject: IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/9859/ : 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/18061
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I408a396d40d9622f2ae6c459f49cbfcc19affd14
Gerrit-Change-Number: 18061
Gerrit-PatchSet: 1
Gerrit-Owner: wangsheng <sk...@163.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: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Wed, 01 Dec 2021 12:26:13 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
wangsheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/18061 )
Change subject: IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
......................................................................
Patch Set 3:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/18061/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18061/2//COMMIT_MSG@17
PS2, Line 17: cause deep nesting.
> nit: cause deep nesting.
Done
http://gerrit.cloudera.org:8080/#/c/18061/2//COMMIT_MSG@22
PS2, Line 22: would execute slowly, or even fail.
> nit: would execute slowly, or even fail.
Done
http://gerrit.cloudera.org:8080/#/c/18061/2/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
File fe/src/main/java/org/apache/impala/analysis/UnionStmt.java:
http://gerrit.cloudera.org:8080/#/c/18061/2/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java@a67
PS2, Line 67:
> I think we probably want this to be executed, we just don't want it to be e
Hi Zoltan, thanks for your patient review, but I don't understand. Do you mean like this?
@Override
public void reset() {
super.reset();
if (resetWasInvoked_) return;
//......
}
Here are some things that confuse me:
1.Why 'we probably want this to be executed'? I aggree that maybe we should keep 'UnionStmt.reset()' like this for future needs:
@Override
public void reset() {
super.reset();
}
But all codes after 'super.reset();' are included in 'SetOperationStmt.reset()', and these memebers are exists in 'SetOperationStmt' which are not been inherited by other sub-class, which means we do the same reset twice in 'UnionStmt.reset()' and 'SetOperationStmt.reset()'.
2.You mentioned that set 'resetWasInvoked_' as false in 'analyze()', when do we set as true? default value as true? In 'reAnalyze()' function, Impala first execute 'reset()', then 'analyze()'. If we set false in 'analyze()'. Before 'reAnalyze()' phase, Impala already execute 'analyze()', 'resetWasInvoked_' was set as false, and this check invalid.
Hope for your reply again!
http://gerrit.cloudera.org:8080/#/c/18061/2/testdata/workloads/functional-query/queries/QueryTest/union.test
File testdata/workloads/functional-query/queries/QueryTest/union.test:
http://gerrit.cloudera.org:8080/#/c/18061/2/testdata/workloads/functional-query/queries/QueryTest/union.test@1187
PS2, Line 1187: would execute slowly, or even fai
> nit: would execute slowly, or even fail.
Done
--
To view, visit http://gerrit.cloudera.org:8080/18061
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I408a396d40d9622f2ae6c459f49cbfcc19affd14
Gerrit-Change-Number: 18061
Gerrit-PatchSet: 3
Gerrit-Owner: wangsheng <sk...@163.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, 08 Dec 2021 09:58:17 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/18061 )
Change subject: IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/18061
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I408a396d40d9622f2ae6c459f49cbfcc19affd14
Gerrit-Change-Number: 18061
Gerrit-PatchSet: 5
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@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: Thu, 09 Dec 2021 16:56:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
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/18061 )
Change subject: IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
......................................................................
IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
When query contains multiple nested union stmt, more than twenty or
thirty, and needs 'reAnalyze', such as rewrite expr. Query would
execute slowly, even failed due to 'reset' method called in class
'UnionStmt' and 'SetOperationStmt'.
'SetOperationStmt' is added in IMPALA-9943 and IMPALA-4974. Multiple
nested union stmt will lead to 'reset' called numbers to grow
exponentially. Since 'operands_' will be reset in two class' reset()
method, and handle with their children recursively. Too many nested
union stmt will cause deep nesting.
UnionStmt.reset() content is exactly same as SetOperationStmt.reset().
This patch removed this method in 'UnionStmt'. After this, the
original query would execute quickly.
An example already add in file 'union.test', without this patch, the
example query would execute slowly, or even fail.
Testing:
- Added new test case in 'union.test'
Change-Id: I408a396d40d9622f2ae6c459f49cbfcc19affd14
Reviewed-on: http://gerrit.cloudera.org:8080/18061
Reviewed-by: Qifan Chen <qc...@cloudera.com>
Reviewed-by: Zoltan Borok-Nagy <bo...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M testdata/workloads/functional-query/queries/QueryTest/union.test
2 files changed, 129 insertions(+), 20 deletions(-)
Approvals:
Qifan Chen: Looks good to me, but someone else must approve
Zoltan Borok-Nagy: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/18061
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I408a396d40d9622f2ae6c459f49cbfcc19affd14
Gerrit-Change-Number: 18061
Gerrit-PatchSet: 6
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
[Impala-ASF-CR] IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
Posted by "wangsheng (Code Review)" <ge...@cloudera.org>.
wangsheng has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/18061 )
Change subject: IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
......................................................................
IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
When query contains multiple nested union stmt, more than twenty or
thirty, and needs 'reAnalyze', such as rewrite expr. Query would
execute slowly, even failed due to 'reset' method called in class
'UnionStmt' and 'SetOperationStmt'.
'SetOperationStmt' is added in IMPALA-9943 and IMPALA-4974. Multiple
nested union stmt will lead to 'reset' called numbers to grow
exponentially. Since 'operands_' will be reset in two class' reset()
method, and handle with their children recursively. Too many nested
union stmt will cause deep nesting.
UnionStmt.reset() content is almost same as SetOperationStmt.reset().
This patch removed this method in 'UnionStmt'. After this, the
original query would execute quickly.
An example already add in file 'union.test', without this patch, the
example query would execute slowly, or even fail.
Testing:
- Added new test case in 'union.test'
Change-Id: I408a396d40d9622f2ae6c459f49cbfcc19affd14
---
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M testdata/workloads/functional-query/queries/QueryTest/union.test
2 files changed, 129 insertions(+), 20 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/18061/3
--
To view, visit http://gerrit.cloudera.org:8080/18061
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I408a396d40d9622f2ae6c459f49cbfcc19affd14
Gerrit-Change-Number: 18061
Gerrit-PatchSet: 3
Gerrit-Owner: wangsheng <sk...@163.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>
[Impala-ASF-CR] IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18061 )
Change subject: IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/9891/ : 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/18061
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I408a396d40d9622f2ae6c459f49cbfcc19affd14
Gerrit-Change-Number: 18061
Gerrit-PatchSet: 3
Gerrit-Owner: wangsheng <sk...@163.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, 08 Dec 2021 10:21:17 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18061 )
Change subject: IMPALA-11040: Remove unnecessary reset() method in class 'UnionStmt'
......................................................................
Patch Set 5:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7699/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/18061
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I408a396d40d9622f2ae6c459f49cbfcc19affd14
Gerrit-Change-Number: 18061
Gerrit-PatchSet: 5
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@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: Thu, 09 Dec 2021 16:57:13 +0000
Gerrit-HasComments: No