You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Xiaoqing Gao (Code Review)" <ge...@cloudera.org> on 2020/12/31 06:25:59 UTC
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be apply to view table
Xiaoqing Gao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16912
Change subject: IMPALA-10412: ConvertToCNFRule can be apply to view table
......................................................................
IMPALA-10412: ConvertToCNFRule can be apply to view table
if the predicate associated more than one tuples and the predicate can be pushed down to scan node
Testing:
Add planner test in inline-view.test
Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
---
M fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
2 files changed, 36 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/12/16912/1
--
To view, visit http://gerrit.cloudera.org:8080/16912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
Gerrit-Change-Number: 16912
Gerrit-PatchSet: 1
Gerrit-Owner: Xiaoqing Gao <ga...@gmail.com>
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be apply to view table
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16912 )
Change subject: IMPALA-10412: ConvertToCNFRule can be apply to view table
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/7919/ : 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/16912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
Gerrit-Change-Number: 16912
Gerrit-PatchSet: 4
Gerrit-Owner: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 31 Dec 2020 09:00:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be applied to view table
Posted by "Xiaoqing Gao (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16912
to look at the new patch set (#7).
Change subject: IMPALA-10412: ConvertToCNFRule can be applied to view table
......................................................................
IMPALA-10412: ConvertToCNFRule can be applied to view table
For OR predicates that reference a view, currently the ConvertToCNFRule
does not get applied since it is considered a single table predicate
even if the predicate might reference columns from different tables
within the view. This patch enables the application of this rule for
such predicates by checking the expanded view and if it satisfies the
criterion then the rule can be applied and the predicate can be pushed
eventually to the scan.
Testing:
Added planner test in inline-view.test
Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
---
M fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
2 files changed, 81 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/12/16912/7
--
To view, visit http://gerrit.cloudera.org:8080/16912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
Gerrit-Change-Number: 16912
Gerrit-PatchSet: 7
Gerrit-Owner: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xiaoqing Gao <ga...@gmail.com>
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be apply to view table
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16912 )
Change subject: IMPALA-10412: ConvertToCNFRule can be apply to view table
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/7918/ : 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/16912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
Gerrit-Change-Number: 16912
Gerrit-PatchSet: 3
Gerrit-Owner: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 31 Dec 2020 08:57:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be applied to view table
Posted by "Xiaoqing Gao (Code Review)" <ge...@cloudera.org>.
Xiaoqing Gao has posted comments on this change. ( http://gerrit.cloudera.org:8080/16912 )
Change subject: IMPALA-10412: ConvertToCNFRule can be applied to view table
......................................................................
Patch Set 6:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/16912/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16912/5//COMMIT_MSG@7
PS5, Line 7: applied to view table
:
> nit: 'applied to a view'
Done
http://gerrit.cloudera.org:8080/#/c/16912/5//COMMIT_MSG@9
PS5, Line 9: For OR predicates that reference a view, currently the ConvertToCNFRule
> This sentence seems to be a continuation of the title summary. In general,
Done
http://gerrit.cloudera.org:8080/#/c/16912/5//COMMIT_MSG@13
PS5, Line 13: suc
> nit: 'Added'
Done
http://gerrit.cloudera.org:8080/#/c/16912/5/fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java
File fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java:
http://gerrit.cloudera.org:8080/#/c/16912/5/fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java@122
PS5, Line 122: if (tbl instanceof InlineViewRef) {
> tids.get(0) could hit index out of bounds if tids was empty .. which is a c
Done
http://gerrit.cloudera.org:8080/#/c/16912/5/fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java@124
PS5, Line 124: analyzer, false);
> I think this should use 'cpred' instead of pred since cpred is the the anal
Done
http://gerrit.cloudera.org:8080/#/c/16912/5/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
File testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test:
http://gerrit.cloudera.org:8080/#/c/16912/5/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test@2487
PS5, Line 2487: appli
> nit: 'applied'
Done
http://gerrit.cloudera.org:8080/#/c/16912/5/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test@2491
PS5, Line 2491: select count(*) from t where t.test_zip = 1 or (t.test_name='xyz' and t.zip=1);
> Could you also add a test where the inline view itself is joined with anoth
Done
http://gerrit.cloudera.org:8080/#/c/16912/5/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test@2515
PS5, Line 2515: # IMPALA-10412: ConvertToCNFRule can be applied to view table
> It wasn't clear what the purpose of this test was. If it is not really exer
Done
--
To view, visit http://gerrit.cloudera.org:8080/16912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
Gerrit-Change-Number: 16912
Gerrit-PatchSet: 6
Gerrit-Owner: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Comment-Date: Mon, 04 Jan 2021 08:12:12 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be apply to view table
Posted by "Xiaoqing Gao (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16912
to look at the new patch set (#4).
Change subject: IMPALA-10412: ConvertToCNFRule can be apply to view table
......................................................................
IMPALA-10412: ConvertToCNFRule can be apply to view table
if the predicate associated more than one tuples and the predicate can
be pushed down to scan node
Testing:
Add planner test in inline-view.test
Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
---
M fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
2 files changed, 55 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/12/16912/4
--
To view, visit http://gerrit.cloudera.org:8080/16912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
Gerrit-Change-Number: 16912
Gerrit-PatchSet: 4
Gerrit-Owner: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be applied to view table
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16912 )
Change subject: IMPALA-10412: ConvertToCNFRule can be applied to view table
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/7963/ : 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/16912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
Gerrit-Change-Number: 16912
Gerrit-PatchSet: 7
Gerrit-Owner: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Comment-Date: Wed, 06 Jan 2021 09:35:19 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be apply to view table
Posted by "Xiaoqing Gao (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16912
to look at the new patch set (#3).
Change subject: IMPALA-10412: ConvertToCNFRule can be apply to view table
......................................................................
IMPALA-10412: ConvertToCNFRule can be apply to view table
if the predicate associated more than one tuples and the predicate can
be pushed down to scan node
Testing:
Add planner test in inline-view.test
Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
---
M fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
2 files changed, 55 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/12/16912/3
--
To view, visit http://gerrit.cloudera.org:8080/16912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
Gerrit-Change-Number: 16912
Gerrit-PatchSet: 3
Gerrit-Owner: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be apply to view table
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16912 )
Change subject: IMPALA-10412: ConvertToCNFRule can be apply to view table
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/7916/ : 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/16912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
Gerrit-Change-Number: 16912
Gerrit-PatchSet: 1
Gerrit-Owner: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 31 Dec 2020 06:47:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be applied to view table
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16912 )
Change subject: IMPALA-10412: ConvertToCNFRule can be applied to view table
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/7934/ : 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/16912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
Gerrit-Change-Number: 16912
Gerrit-PatchSet: 6
Gerrit-Owner: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Comment-Date: Mon, 04 Jan 2021 08:33:42 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be apply to view table
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16912 )
Change subject: IMPALA-10412: ConvertToCNFRule can be apply to view table
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/7920/ : 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/16912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
Gerrit-Change-Number: 16912
Gerrit-PatchSet: 5
Gerrit-Owner: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 31 Dec 2020 09:01:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be applied to view table
Posted by "Xiaoqing Gao (Code Review)" <ge...@cloudera.org>.
Xiaoqing Gao has posted comments on this change. ( http://gerrit.cloudera.org:8080/16912 )
Change subject: IMPALA-10412: ConvertToCNFRule can be applied to view table
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16912/5/fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java
File fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java:
http://gerrit.cloudera.org:8080/#/c/16912/5/fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java@126
PS5, Line 126: tids.clear();
> If the predicate did not change based on the previous trySubstitute() call,
Done
--
To view, visit http://gerrit.cloudera.org:8080/16912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
Gerrit-Change-Number: 16912
Gerrit-PatchSet: 5
Gerrit-Owner: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Comment-Date: Mon, 04 Jan 2021 08:12:40 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be applied to view table
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/16912 )
Change subject: IMPALA-10412: ConvertToCNFRule can be applied to view table
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16912/6/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
File testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test:
http://gerrit.cloudera.org:8080/#/c/16912/6/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test@2520
PS6, Line 2520: AND t.test_zip = 1 or (t.test_name='xyz' and t.zip=1);
My example query mistakenly omitted the parenthesis after the AND predicate..that's why you are getting a nested loop join with all the OR predicates. You may want to change this line to:
AND (t.test_zip = 1 or (t.test_name='xyz' and t.zip=1));
This will ensure that you have a Hash Join with the equijoin predicate and the rest of the predicates can be pushed to the view.
--
To view, visit http://gerrit.cloudera.org:8080/16912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
Gerrit-Change-Number: 16912
Gerrit-PatchSet: 6
Gerrit-Owner: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Comment-Date: Tue, 05 Jan 2021 00:47:18 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be applied to view table
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/16912 )
Change subject: IMPALA-10412: ConvertToCNFRule can be applied to view table
......................................................................
Patch Set 6: Code-Review+1
Other than the one remaining comment about the test case, I am good with this patch, so giving it a +1. Once you update the test case, I can change it to +2.
--
To view, visit http://gerrit.cloudera.org:8080/16912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
Gerrit-Change-Number: 16912
Gerrit-PatchSet: 6
Gerrit-Owner: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Comment-Date: Wed, 06 Jan 2021 07:18:34 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be apply to view table
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/16912 )
Change subject: IMPALA-10412: ConvertToCNFRule can be apply to view table
......................................................................
Patch Set 5:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/16912/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16912/5//COMMIT_MSG@7
PS5, Line 7: apply to view table
:
nit: 'applied to a view'
http://gerrit.cloudera.org:8080/#/c/16912/5//COMMIT_MSG@9
PS5, Line 9: if the predicate associated more than one tuples and the predicate can
This sentence seems to be a continuation of the title summary. In general, it is preferred to have the full independent description here. Something like...'For OR predicates that reference a view, currently the ConvertToCNFRule does not get applied since it is considered a single table predicate even if the predicate might reference columns from different tables within the view. This patch enables the application of this rule for such predicates by checking the expanded view and if it satisfies the criterion then the rule can be applied and the predicate can be pushed eventually to the scan.
http://gerrit.cloudera.org:8080/#/c/16912/5//COMMIT_MSG@13
PS5, Line 13: Add
nit: 'Added'
http://gerrit.cloudera.org:8080/#/c/16912/5/fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java
File fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java:
http://gerrit.cloudera.org:8080/#/c/16912/5/fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java@122
PS5, Line 122: TableRef tbl = analyzer.getTableRef(tids.get(0));
tids.get(0) could hit index out of bounds if tids was empty .. which is a corner case but since the if condition above allows size <=1, it is still possible. You can move this logic outside this check and just enclose it within a :
'if (tids.size() == 1)' check.
http://gerrit.cloudera.org:8080/#/c/16912/5/fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java@124
PS5, Line 124: Expr basePred = pred.trySubstitute(((InlineViewRef)tbl).getBaseTblSmap(),
I think this should use 'cpred' instead of pred since cpred is the the analyzed (and cloned) version .. see above on line 114.
http://gerrit.cloudera.org:8080/#/c/16912/5/fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java@126
PS5, Line 126: tids.clear();
If the predicate did not change based on the previous trySubstitute() call, then it is better to skip clearing/re-populating the tids.
http://gerrit.cloudera.org:8080/#/c/16912/5/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
File testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test:
http://gerrit.cloudera.org:8080/#/c/16912/5/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test@2487
PS5, Line 2487: apply
nit: 'applied'
http://gerrit.cloudera.org:8080/#/c/16912/5/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test@2491
PS5, Line 2491: select count(*) from t where t.test_zip = 1 or (t.test_name='xyz' and t.zip=1);
Could you also add a test where the inline view itself is joined with another table ? e.g
select count(*) from t, functional.alltypes other_table where t.test_id = other_table.id
AND t.test_zip = 1 or (t.test_name='xyz' and t.zip=1) ;
In this case, you would expect the CNF to be applied
http://gerrit.cloudera.org:8080/#/c/16912/5/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test@2515
PS5, Line 2515: # IMPALA-10412: ConvertToCNFRule can be apply to view table
It wasn't clear what the purpose of this test was. If it is not really exercising the rule, you could skip it.
--
To view, visit http://gerrit.cloudera.org:8080/16912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
Gerrit-Change-Number: 16912
Gerrit-PatchSet: 5
Gerrit-Owner: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sat, 02 Jan 2021 07:13:30 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be applied to view table
Posted by "Xiaoqing Gao (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16912
to look at the new patch set (#6).
Change subject: IMPALA-10412: ConvertToCNFRule can be applied to view table
......................................................................
IMPALA-10412: ConvertToCNFRule can be applied to view table
For OR predicates that reference a view, currently the ConvertToCNFRule
does not get applied since it is considered a single table predicate
even if the predicate might reference columns from different tables
within the view. This patch enables the application of this rule for
such predicates by checking the expanded view and if it satisfies the
criterion then the rule can be applied and the predicate can be pushed
eventually to the scan.
Testing:
Added planner test in inline-view.test
Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
---
M fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
2 files changed, 78 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/12/16912/6
--
To view, visit http://gerrit.cloudera.org:8080/16912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
Gerrit-Change-Number: 16912
Gerrit-PatchSet: 6
Gerrit-Owner: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be applied to view table
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/16912 )
Change subject: IMPALA-10412: ConvertToCNFRule can be applied to view table
......................................................................
IMPALA-10412: ConvertToCNFRule can be applied to view table
For OR predicates that reference a view, currently the ConvertToCNFRule
does not get applied since it is considered a single table predicate
even if the predicate might reference columns from different tables
within the view. This patch enables the application of this rule for
such predicates by checking the expanded view and if it satisfies the
criterion then the rule can be applied and the predicate can be pushed
eventually to the scan.
Testing:
Added planner test in inline-view.test
Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
Reviewed-on: http://gerrit.cloudera.org:8080/16912
Reviewed-by: Aman Sinha <am...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
2 files changed, 81 insertions(+), 1 deletion(-)
Approvals:
Aman Sinha: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/16912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
Gerrit-Change-Number: 16912
Gerrit-PatchSet: 8
Gerrit-Owner: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xiaoqing Gao <ga...@gmail.com>
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be applied to view table
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/16912 )
Change subject: IMPALA-10412: ConvertToCNFRule can be applied to view table
......................................................................
Patch Set 6:
Also, it would be good if you could rebase on master since there's a merge conflict.
--
To view, visit http://gerrit.cloudera.org:8080/16912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
Gerrit-Change-Number: 16912
Gerrit-PatchSet: 6
Gerrit-Owner: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Comment-Date: Wed, 06 Jan 2021 07:21:15 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be applied to view table
Posted by "Xiaoqing Gao (Code Review)" <ge...@cloudera.org>.
Xiaoqing Gao has posted comments on this change. ( http://gerrit.cloudera.org:8080/16912 )
Change subject: IMPALA-10412: ConvertToCNFRule can be applied to view table
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16912/6/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
File testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test:
http://gerrit.cloudera.org:8080/#/c/16912/6/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test@2520
PS6, Line 2520: | row-size=1B cardinality=9
> My example query mistakenly omitted the parenthesis after the AND predicate
Done
--
To view, visit http://gerrit.cloudera.org:8080/16912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
Gerrit-Change-Number: 16912
Gerrit-PatchSet: 7
Gerrit-Owner: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Comment-Date: Wed, 06 Jan 2021 09:15:33 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be apply to view table
Posted by "Xiaoqing Gao (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16912
to look at the new patch set (#2).
Change subject: IMPALA-10412: ConvertToCNFRule can be apply to view table
......................................................................
IMPALA-10412: ConvertToCNFRule can be apply to view table
if the predicate associated more than one tuples and the predicate can
be pushed down to scan node
Testing:
Add planner test in inline-view.test
Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
---
M fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
2 files changed, 39 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/12/16912/2
--
To view, visit http://gerrit.cloudera.org:8080/16912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
Gerrit-Change-Number: 16912
Gerrit-PatchSet: 2
Gerrit-Owner: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be apply to view table
Posted by "Xiaoqing Gao (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16912
to look at the new patch set (#5).
Change subject: IMPALA-10412: ConvertToCNFRule can be apply to view table
......................................................................
IMPALA-10412: ConvertToCNFRule can be apply to view table
if the predicate associated more than one tuples and the predicate can
be pushed down to scan node
Testing:
Add planner test in inline-view.test
Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
---
M fe/src/main/java/org/apache/impala/rewrite/ConvertToCNFRule.java
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
2 files changed, 55 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/12/16912/5
--
To view, visit http://gerrit.cloudera.org:8080/16912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
Gerrit-Change-Number: 16912
Gerrit-PatchSet: 5
Gerrit-Owner: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be applied to view table
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16912 )
Change subject: IMPALA-10412: ConvertToCNFRule can be applied to view table
......................................................................
Patch Set 7: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/16912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
Gerrit-Change-Number: 16912
Gerrit-PatchSet: 7
Gerrit-Owner: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Comment-Date: Wed, 06 Jan 2021 22:36:09 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be apply to view table
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16912 )
Change subject: IMPALA-10412: ConvertToCNFRule can be apply to view table
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/7917/ : 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/16912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
Gerrit-Change-Number: 16912
Gerrit-PatchSet: 2
Gerrit-Owner: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 31 Dec 2020 06:59:23 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be applied to view table
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16912 )
Change subject: IMPALA-10412: ConvertToCNFRule can be applied to view table
......................................................................
Patch Set 7:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6829/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/16912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
Gerrit-Change-Number: 16912
Gerrit-PatchSet: 7
Gerrit-Owner: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Comment-Date: Wed, 06 Jan 2021 16:49:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be applied to view table
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/16912 )
Change subject: IMPALA-10412: ConvertToCNFRule can be applied to view table
......................................................................
Patch Set 7: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/16912
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie7a9a215d6b92aec07153e643268370f34186c88
Gerrit-Change-Number: 16912
Gerrit-PatchSet: 7
Gerrit-Owner: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Xiaoqing Gao <ga...@gmail.com>
Gerrit-Comment-Date: Wed, 06 Jan 2021 16:48:31 +0000
Gerrit-HasComments: No