[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be apply to view table
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 Gerrit-Reviewer: Aman Sinha Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Sat, 02 Jan 2021 07:13:30 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be apply to view table
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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 31 Dec 2020 09:01:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be apply to view table
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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 31 Dec 2020 09:00:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be apply to view table
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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 31 Dec 2020 08:57:25 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be apply to view table
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 Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be apply to view table
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 Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be apply to view table
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 Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be apply to view table
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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 31 Dec 2020 06:59:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be apply to view table
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 Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 31 Dec 2020 06:47:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10412: ConvertToCNFRule can be apply to view table
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 Gerrit-Reviewer: Impala Public Jenkins
[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
