You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Aman Sinha (Code Review)" <ge...@cloudera.org> on 2019/12/01 19:04:13 UTC
[Impala-ASF-CR] IMPALA-9162: Do not apply new inferred predicate to outer joins
Aman Sinha has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14813
Change subject: IMPALA-9162: Do not apply new inferred predicate to outer joins
......................................................................
IMPALA-9162: Do not apply new inferred predicate to outer joins
When the planner migrates predicates to inline views, it also creates
equivalent predicates based on the value transfer graph which is built
by transitive relationships among join conditions. These newly inferred
predicates are placed typically as 'other predicates' of an inner or
outer join.
However, for outer joins, this has the effect of adding extra predicates
in the WHERE clause which is incorrect since it may filter NULL values.
Since the original query did not have null filtering conditions in
the WHERE clause, we should not add new ones. This fix prevents it by
analyzing the new inferred predicate of type A <op> B and if either the
left or right slots reference the output tuple of an outer join, the
inferred predicate is ignored.
Note that simple queries with combination of inner and outer joins may
not reproduce the problem. Due to the nature of predicate inferencing,
some combination of subqueries, inner joins, outer joins is needed. For
the query pattern, please see the example in the JIRA.
Tests:
- Added plan tests with left and right outer joins to inline-view.test
- Manually ran few queries on impala shell to verify result
correctness: by checking that NULL values are being produced for outer
joins.
- Ran regression tests on jenkins
Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
---
M fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
3 files changed, 131 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/14813/1
--
To view, visit http://gerrit.cloudera.org:8080/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 1
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
[Impala-ASF-CR] IMPALA-9162: Do not apply inferred predicate to outer joins
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Tim Armstrong, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14813
to look at the new patch set (#5).
Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................
IMPALA-9162: Do not apply inferred predicate to outer joins
When the planner migrates predicates to inline views, it also creates
equivalent predicates based on the value transfer graph which is built
by transitive relationships among join conditions. These newly inferred
predicates are placed typically as 'other predicates' of an inner or
outer join.
However, for outer joins, this has the effect of adding extra predicates
in the WHERE clause which is incorrect since it may filter NULL values.
Since the original query did not have null filtering conditions in
the WHERE clause, we should not add new ones. In this fix we do the
following: during the migration of conjuncts to inline views, analyze
the predicate of type A <op> B and if it is an inferred predicate AND
either the left or right slots reference the output tuple of an outer
join, the inferred predicate is ignored.
Note that simple queries with combination of inner and outer joins may
not reproduce the problem. Due to the nature of predicate inferencing,
some combination of subqueries, inner joins, outer joins is needed. For
the query pattern, please see the example in the JIRA.
Tests:
- Added plan tests with left and right outer joins to inline-view.test
- One baseline plan in inline-view.test had to be updated
- Manually ran few queries on impala shell to verify result
correctness: by checking that NULL values are being produced for outer
joins.
- Ran regression tests on jenkins
Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
---
M fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
3 files changed, 143 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/14813/5
--
To view, visit http://gerrit.cloudera.org:8080/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 5
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-9162: Do not apply inferred predicate to outer joins
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/14813 )
Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................
Patch Set 3:
Quanlong, I am seeing a few PlannerTest failures. I am looking into it, so you may want to hold off on the follow-up review.
--
To view, visit http://gerrit.cloudera.org:8080/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 06 Dec 2019 18:46:13 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9162: Do not apply inferred predicate to outer joins
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14813 )
Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................
Patch Set 7: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 7
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Dec 2019 05:48:52 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9162: Do not apply inferred predicate to outer joins
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14813 )
Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14813/6/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java:
http://gerrit.cloudera.org:8080/#/c/14813/6/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1201
PS6, Line 1201: analyzer.markConjunctsAssigned(preds);
> I can certainly update the comments regarding it being safe to drop inferre
Oh, of course. I missed that they were generated there. I guess a comment is sufficient then.
--
To view, visit http://gerrit.cloudera.org:8080/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 6
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Dec 2019 19:50:37 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9162: Do not apply inferred predicate to outer joins
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14813 )
Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5226/ : 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/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 5
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Dec 2019 03:35:30 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9162: Do not apply inferred predicate to outer joins
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Tim Armstrong, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14813
to look at the new patch set (#4).
Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................
IMPALA-9162: Do not apply inferred predicate to outer joins
When the planner migrates predicates to inline views, it also creates
equivalent predicates based on the value transfer graph which is built
by transitive relationships among join conditions. These newly inferred
predicates are placed typically as 'other predicates' of an inner or
outer join.
However, for outer joins, this has the effect of adding extra predicates
in the WHERE clause which is incorrect since it may filter NULL values.
Since the original query did not have null filtering conditions in
the WHERE clause, we should not add new ones. In this fix we do the
following: during the migration of conjuncts to inline views, analyze
the predicate of type A <op> B and if it is an inferred predicate AND
either the left or right slots reference the output tuple of an outer
join, the inferred predicate is ignored.
Note that simple queries with combination of inner and outer joins may
not reproduce the problem. Due to the nature of predicate inferencing,
some combination of subqueries, inner joins, outer joins is needed. For
the query pattern, please see the example in the JIRA.
Tests:
- Added plan tests with left and right outer joins to inline-view.test
- One baseline plan in inline-view.test had to be updated
- Manually ran few queries on impala shell to verify result
correctness: by checking that NULL values are being produced for outer
joins.
- Ran regression tests on jenkins
Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
---
M fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
3 files changed, 144 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/14813/4
--
To view, visit http://gerrit.cloudera.org:8080/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 4
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-9162: Do not apply inferred predicate to outer joins
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14813 )
Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................
Patch Set 4:
(7 comments)
I'm agree to fix the bug first since it's emergent. Leave some minor commments about the implemenetation.
http://gerrit.cloudera.org:8080/#/c/14813/4/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
File fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java:
http://gerrit.cloudera.org:8080/#/c/14813/4/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java@333
PS4, Line 333: public static Pair<SlotRef, SlotRef> getEqSlotRefs(Expr e) {
This function looks unused.
http://gerrit.cloudera.org:8080/#/c/14813/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java:
http://gerrit.cloudera.org:8080/#/c/14813/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1270
PS4, Line 1270: InlineViewRef viewRef
Looks like we just need the Analyzer.
http://gerrit.cloudera.org:8080/#/c/14813/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1276
PS4, Line 1276: getSlotRefs
Why not using getEqSlots instead since inferred predicates generated in migrateConjunctsToInlineView are all equivalent predicates?
http://gerrit.cloudera.org:8080/#/c/14813/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1280
PS4, Line 1280: join
nit: joined
http://gerrit.cloudera.org:8080/#/c/14813/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1281
PS4, Line 1281: if (viewRef.getAnalyzer().isOuterJoined(leftParent) ||
: viewRef.getAnalyzer().isOuterJoined(rightParent)) {
I think it could be right if only one part is outer joined. But since this is an inferred predicate, I'm agree to be conservative. Could you add a comment about this?
http://gerrit.cloudera.org:8080/#/c/14813/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1283
PS4, Line 1283: iter.remove();
Can we log a WARNING here? So we know something is wrong before here causing us infer such a predicate.
http://gerrit.cloudera.org:8080/#/c/14813/4/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/14813/4/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test@2271
PS4, Line 2271: right
There are no predicates can be incorrectly assigned to "t". What about changing this to "full outer join"?
--
To view, visit http://gerrit.cloudera.org:8080/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 4
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Dec 2019 02:14:33 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9162: Do not apply inferred predicate to outer joins
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14813 )
Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................
Patch Set 7:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5326/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 7
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Dec 2019 01:19:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9162: Do not apply inferred predicate to outer joins
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/14813 )
Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................
Patch Set 5:
(7 comments)
Thanks for the follow-up review. I have addressed your review comments.
http://gerrit.cloudera.org:8080/#/c/14813/4/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
File fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java:
http://gerrit.cloudera.org:8080/#/c/14813/4/fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java@333
PS4, Line 333: public Pair<SlotRef, SlotRef> getEqSlotRefs() {
> This function looks unused.
I have removed it in the updated patch and instead check for EQ in the non-static function.
http://gerrit.cloudera.org:8080/#/c/14813/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java:
http://gerrit.cloudera.org:8080/#/c/14813/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1270
PS4, Line 1270: Analyzer analyzer, Li
> Looks like we just need the Analyzer.
Done
http://gerrit.cloudera.org:8080/#/c/14813/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1276
PS4, Line 1276: getEqSlotRe
> Why not using getEqSlots instead since inferred predicates generated in mig
Changed this to use the getEqSlotRefs() (I need the 'SlotRefs', not just the SlotId)
http://gerrit.cloudera.org:8080/#/c/14813/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1280
PS4, Line 1280: join
> nit: joined
Done
http://gerrit.cloudera.org:8080/#/c/14813/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1281
PS4, Line 1281: // Note: strictly, we may be ok to check only for the null producing
: // side but we are being conservative here to check both si
> I think it could be right if only one part is outer joined. But since this
Done
http://gerrit.cloudera.org:8080/#/c/14813/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1283
PS4, Line 1283: // additional testing we could potentially relax this.
> Can we log a WARNING here? So we know something is wrong before here causin
Done
http://gerrit.cloudera.org:8080/#/c/14813/4/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/14813/4/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test@2271
PS4, Line 2271: full
> There are no predicates can be incorrectly assigned to "t". What about chan
Here 't' is the null producing side, right ? In any case, I changed this to Full Outer Join since that gives broader coverage.
--
To view, visit http://gerrit.cloudera.org:8080/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 5
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Dec 2019 03:18:03 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9162: Do not apply inferred predicate to outer joins
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/14813 )
Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14813/6/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java:
http://gerrit.cloudera.org:8080/#/c/14813/6/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1201
PS6, Line 1201: analyzer.markConjunctsAssigned(preds);
> Oh, of course. I missed that they were generated there. I guess a comment i
I should note that I tried adding additional check in Analyzer.createEquivConjuncts() in order to catch the point where the inferred predicate is created and prevent its creation if its SlotRef is coming from a Tuple which is an outer joined tuple. However, that did not help and the reason is that in that method, isOuterJoined(leftTupleId) || isOuterJoined(rightTupleId) return FALSE for the problem query pattern. Only after the inline view's substitution map gets used in this method we get the new viewPredicates (line 1243 below) and for these the isOuterJoined(left/right tuple id) returns TRUE.
--
To view, visit http://gerrit.cloudera.org:8080/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 6
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Dec 2019 01:02:50 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9162: Do not apply inferred predicate to outer joins
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Tim Armstrong, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14813
to look at the new patch set (#6).
Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................
IMPALA-9162: Do not apply inferred predicate to outer joins
When the planner migrates predicates to inline views, it also creates
equivalent predicates based on the value transfer graph which is built
by transitive relationships among join conditions. These newly inferred
predicates are placed typically as 'other predicates' of an inner or
outer join.
However, for outer joins, this has the effect of adding extra predicates
in the WHERE clause which is incorrect since it may filter NULL values.
Since the original query did not have null filtering conditions in
the WHERE clause, we should not add new ones. In this fix we do the
following: during the migration of conjuncts to inline views, analyze
the predicate of type A <op> B and if it is an inferred predicate AND
either the left or right slots reference the output tuple of an outer
join, the inferred predicate is ignored.
Note that simple queries with combination of inner and outer joins may
not reproduce the problem. Due to the nature of predicate inferencing,
some combination of subqueries, inner joins, outer joins is needed. For
the query pattern, please see the example in the JIRA.
Tests:
- Added plan tests with left and right outer joins to inline-view.test
- One baseline plan in inline-view.test had to be updated
- Manually ran few queries on impala shell to verify result
correctness: by checking that NULL values are being produced for outer
joins.
- Ran regression tests on jenkins
Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
---
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
2 files changed, 130 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/14813/6
--
To view, visit http://gerrit.cloudera.org:8080/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 6
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-9162: Do not apply inferred predicate to outer joins
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14813 )
Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 6
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Dec 2019 01:19:29 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9162: Do not apply inferred predicate to outer joins
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14813 )
Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................
Patch Set 2:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/14813/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/14813/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2238
PS2, Line 2238: LOG.trace("Considering value transfer between " + slotRefs.first.getSlotId().toString() +
line too long (99 > 90)
http://gerrit.cloudera.org:8080/#/c/14813/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2244
PS2, Line 2244: g.addEdge(slotRefs.first.getSlotId().asInt(), slotRefs.second.getSlotId().asInt());
line too long (93 > 90)
http://gerrit.cloudera.org:8080/#/c/14813/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2246
PS2, Line 2246: LOG.trace("value transfer: from " + slotRefs.first.getSlotId().toString() + " to " +
line too long (96 > 90)
http://gerrit.cloudera.org:8080/#/c/14813/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2253
PS2, Line 2253: g.addEdge(slotRefs.second.getSlotId().asInt(), slotRefs.first.getSlotId().asInt());
line too long (93 > 90)
http://gerrit.cloudera.org:8080/#/c/14813/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2255
PS2, Line 2255: LOG.trace("value transfer: from " + slotRefs.second.getSlotId().toString() + " to " +
line too long (97 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 2
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 06 Dec 2019 02:27:08 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9162: Do not apply inferred predicate to outer joins
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/14813 )
Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................
Patch Set 3:
Quanlong, I have uploaded additional changes after addressing review comments. I have kept the original fix plus fixed the values transfer issue you found. Could you pls take another look ? Thanks.
--
To view, visit http://gerrit.cloudera.org:8080/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 06 Dec 2019 02:40:21 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9162: Do not apply inferred predicate to outer joins
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Tim Armstrong, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14813
to look at the new patch set (#2).
Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................
IMPALA-9162: Do not apply inferred predicate to outer joins
When the planner migrates predicates to inline views, it also creates
equivalent predicates based on the value transfer graph which is built
by transitive relationships among join conditions. These newly inferred
predicates are placed typically as 'other predicates' of an inner or
outer join.
However, for outer joins, this has the effect of adding extra predicates
in the WHERE clause which is incorrect since it may filter NULL values.
Since the original query did not have null filtering conditions in
the WHERE clause, we should not add new ones. This fix does 2 things:
- At the time of creating the values transfer graph in Analyzer,
don't add a directed edge between two slots if the first slot
references the output tuple of an outer join.
- During the migration of conjuncts to inline views, analyze the
predicate of type A <op> B and if it is an inferred predicate AND
either the left or right slots reference the output tuple of an outer
join, the inferred predicate is ignored. This serves as a safety
check in case any unqualified predicate 'fell through' until this
stage of planning.
Note that simple queries with combination of inner and outer joins may
not reproduce the problem. Due to the nature of predicate inferencing,
some combination of subqueries, inner joins, outer joins is needed. For
the query pattern, please see the example in the JIRA.
Tests:
- Added plan tests with left and right outer joins to inline-view.test
- Manually ran few queries on impala shell to verify result
correctness: by checking that NULL values are being produced for outer
joins.
- Ran regression tests on jenkins
Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
4 files changed, 169 insertions(+), 21 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/14813/2
--
To view, visit http://gerrit.cloudera.org:8080/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 2
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-9162: Do not apply inferred predicate to outer joins
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/14813 )
Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................
Patch Set 4:
I updated the patch by keeping the original fix (ignoring the inferred predicate for OJs at the time of migration of conjuncts into inline view) since it passes all regression tests. If I add the additional fix for the value transfer graph, there are a few test failures in PlannerTest that will need additional time to investigate (I can share the list of failures offline). Since this JIRA is an escalation, I am inclined to go with the limited/conservative fix to address it and open a separate JIRA for the value transfer. Pls let me know what you think.
--
To view, visit http://gerrit.cloudera.org:8080/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 4
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sun, 08 Dec 2019 23:58:40 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9162: Do not apply inferred predicate to outer joins
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/14813 )
Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14813/6/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java:
http://gerrit.cloudera.org:8080/#/c/14813/6/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1201
PS6, Line 1201: analyzer.markConjunctsAssigned(preds);
> I'm trying to convince myself that it's correct to mark the conjuncts as as
I can certainly update the comments regarding it being safe to drop inferred predicates of certain types. The inferred predicates are only created during the analyzer.createEquivConjuncts() call on line 1204, so we cannot drop them before that. I agree that ideally we should not be generating the inferred conjuncts that don't qualify. I have done a few experiments with doing this sooner (also based on Quanlong's suggestion about value transfer graph ..see a prior review comment). In those cases I found quite a few Explain plan diffs which would take some time to investigate. So, given that this is an escalation, I took a conservative approach. I will do some more analysis.
--
To view, visit http://gerrit.cloudera.org:8080/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 6
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Dec 2019 19:40:40 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9162: Do not apply new inferred predicate to outer joins
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14813 )
Change subject: IMPALA-9162: Do not apply new inferred predicate to outer joins
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5178/ : 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/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 1
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sun, 01 Dec 2019 19:33:29 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9162: Do not apply inferred predicate to outer joins
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/14813 )
Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................
Patch Set 6:
(2 comments)
Updated patch.
http://gerrit.cloudera.org:8080/#/c/14813/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java:
http://gerrit.cloudera.org:8080/#/c/14813/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1283
PS4, Line 1283: // additional testing we could potentially relax this.
> Done
There was also a bug here ..a 'break' statement which should not be there since we want to remove all such invalid predicates. I have removed it. I think I was doing some testing in the past and it got left over.
http://gerrit.cloudera.org:8080/#/c/14813/5/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java:
http://gerrit.cloudera.org:8080/#/c/14813/5/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1276
PS5, Line 1276: Pair<SlotId, SlotId> slots = p.getEqSlots();
> Sorry to be unclear in the last comment. I mean we can use p.getEqSlots() h
Yes, makes sense. I can use the Analyzer's method.
--
To view, visit http://gerrit.cloudera.org:8080/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 6
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Dec 2019 04:09:21 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9162: Do not apply inferred predicate to outer joins
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14813 )
Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14813/5/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java:
http://gerrit.cloudera.org:8080/#/c/14813/5/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1276
PS5, Line 1276: Pair<SlotRef, SlotRef> slotRefs = p.getEqSlotRefs();
Sorry to be unclear in the last comment. I mean we can use p.getEqSlots() here. After getting the SlotIds, get their TupleId by Analyzer#getTupleId(SlotId slotId). Then we don't need to add a new function getEqSlotRefs() in BinaryPredicate.
--
To view, visit http://gerrit.cloudera.org:8080/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 5
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Dec 2019 03:18:16 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9162: Do not apply inferred predicate to outer joins
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14813 )
Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5225/ : 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/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 4
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Dec 2019 00:13:43 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9162: Do not apply inferred predicate to outer joins
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14813 )
Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................
Patch Set 6:
Thanks for taking the time to explain, I think I see what I was missing earlier.
--
To view, visit http://gerrit.cloudera.org:8080/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 6
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Dec 2019 01:19:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9162: Do not apply inferred predicate to outer joins
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14813 )
Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5227/ : 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/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 6
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Dec 2019 04:31:58 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9162: Do not apply new inferred predicate to outer joins
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14813 )
Change subject: IMPALA-9162: Do not apply new inferred predicate to outer joins
......................................................................
Patch Set 1:
(4 comments)
Thank Aman for digging into this! This patch makes sense to me.
While trying to understand why we infer such a predicate, I found a bug in constructing the valueTransferGraph. I left a comment in the JIRA to explain it. (Sorry that JIRA comment is more formattable than Gerrit so I leave it there)
Could you also fix it so we won't have chance to infer wrong predicates?
http://gerrit.cloudera.org:8080/#/c/14813/1/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java:
http://gerrit.cloudera.org:8080/#/c/14813/1/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@29
PS1, Line 29: import java.util.ListIterator;
nit: keep the import list sorted
http://gerrit.cloudera.org:8080/#/c/14813/1/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1266
PS1, Line 1266: *
: * @param viewRef
: * @param preds
nit: don't need these
http://gerrit.cloudera.org:8080/#/c/14813/1/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1273
PS1, Line 1273:
nit: double whitespaces
http://gerrit.cloudera.org:8080/#/c/14813/1/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/14813/1/testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test@2228
PS1, Line 2228:
nit: use whitespaces instead of tabs
--
To view, visit http://gerrit.cloudera.org:8080/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 1
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 05 Dec 2019 07:10:04 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9162: Do not apply new inferred predicate to outer joins
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14813 )
Change subject: IMPALA-9162: Do not apply new inferred predicate to outer joins
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14813/1/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java:
http://gerrit.cloudera.org:8080/#/c/14813/1/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1264
PS1, Line 1264: * the predicate is an inferred predicate AND either its left or right SlotRef references
line too long (91 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 1
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sun, 01 Dec 2019 19:04:51 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9162: Do not apply inferred predicate to outer joins
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14813 )
Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................
Patch Set 6:
(2 comments)
Had a few questions about why this works to help me convince myself that it's correct.
http://gerrit.cloudera.org:8080/#/c/14813/6//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14813/6//COMMIT_MSG@2
PS6, Line 2: Author: Aman Sinha <am...@cloudera.com>
I dunno if this matters, but it seems like you have two different email addresses configured in git (here and l4)
http://gerrit.cloudera.org:8080/#/c/14813/6/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java:
http://gerrit.cloudera.org:8080/#/c/14813/6/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1201
PS6, Line 1201: analyzer.markConjunctsAssigned(preds);
I'm trying to convince myself that it's correct to mark the conjuncts as assigned here if we remove them from the list below. The potential issue is that
I'm not sure what the right option is but I think there's something to clarify or fix. I can think of a few alternatives, I think we probably need to at least update a comment or two.
* Maybe we need to mention somewhere that it's safe to drop inferred predicates (like identity predicates) because the original predicates will still be assigned. I think this is true.
* We could also potentially remove the predicates from 'preds' up here. Does that have other consequences? E.g. the predicate stays around and gets assigned somewhere. Or is the inferred predicate getting removed key to the fix?
* Is is possible we should just not be generating the inferred conjuncts? E.g. Analyzer.createEquivConjuncts(TupleId, List<T>,
Set<SlotId>) should not be creating these.
--
To view, visit http://gerrit.cloudera.org:8080/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 6
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Dec 2019 16:16:46 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9162: Do not apply inferred predicate to outer joins
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/14813 )
Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................
Patch Set 4:
In case you are interested, here's a link to the backup branch which contains both the original fix + fix to the value transfer graph creation : https://github.com/amansinha100/impala/commit/0ad31bc1c0a6effaa58a2be04494c9d04cc26fb8
--
To view, visit http://gerrit.cloudera.org:8080/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 4
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Dec 2019 00:00:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9162: Do not apply inferred predicate to outer joins
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14813 )
Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................
Patch Set 6: Code-Review+1
(1 comment)
Patch looks good to me overall. Will ask Tim to see whether he want to take a look.
http://gerrit.cloudera.org:8080/#/c/14813/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java:
http://gerrit.cloudera.org:8080/#/c/14813/4/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1283
PS4, Line 1283: // additional testing we could potentially relax this.
> There was also a bug here ..a 'break' statement which should not be there s
Ah, I miss it too... So if it's not hard, could you add a test case in which we remove more than one predicate to cover this? Not sure if adding another column or another view (iv3) could achieve this. I'm ok to miss this coverage if it'll take you too much time :)
--
To view, visit http://gerrit.cloudera.org:8080/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 6
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Dec 2019 04:29:18 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9162: Do not apply inferred predicate to outer joins
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14813 )
Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5219/ : 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/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 2
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 06 Dec 2019 02:56:00 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9162: Do not apply inferred predicate to outer joins
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Tim Armstrong, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14813
to look at the new patch set (#3).
Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................
IMPALA-9162: Do not apply inferred predicate to outer joins
When the planner migrates predicates to inline views, it also creates
equivalent predicates based on the value transfer graph which is built
by transitive relationships among join conditions. These newly inferred
predicates are placed typically as 'other predicates' of an inner or
outer join.
However, for outer joins, this has the effect of adding extra predicates
in the WHERE clause which is incorrect since it may filter NULL values.
Since the original query did not have null filtering conditions in
the WHERE clause, we should not add new ones. This fix does 2 things:
- At the time of creating the values transfer graph in Analyzer,
don't add a directed edge between two slots if the first slot
references the output tuple of an outer join.
- During the migration of conjuncts to inline views, analyze the
predicate of type A <op> B and if it is an inferred predicate AND
either the left or right slots reference the output tuple of an outer
join, the inferred predicate is ignored. This serves as a safety
check in case any unqualified predicate 'fell through' until this
stage of planning.
Note that simple queries with combination of inner and outer joins may
not reproduce the problem. Due to the nature of predicate inferencing,
some combination of subqueries, inner joins, outer joins is needed. For
the query pattern, please see the example in the JIRA.
Tests:
- Added plan tests with left and right outer joins to inline-view.test
- Manually ran few queries on impala shell to verify result
correctness: by checking that NULL values are being produced for outer
joins.
- Ran regression tests on jenkins
Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
4 files changed, 172 insertions(+), 21 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/14813/3
--
To view, visit http://gerrit.cloudera.org:8080/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-9162: Do not apply new inferred predicate to outer joins
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/14813 )
Change subject: IMPALA-9162: Do not apply new inferred predicate to outer joins
......................................................................
Patch Set 1:
Thanks Quanlong for the review and the explanation in the JIRA. Just to be clear you are suggesting to keep my current fix and in addition fix the bug in valueTransferGraph creation ? I could probably re-use the logic that I have for ignoring BinaryPredicates whose SlotRef references the output of an Outer Join.
--
To view, visit http://gerrit.cloudera.org:8080/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 1
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 05 Dec 2019 16:49:31 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9162: Do not apply inferred predicate to outer joins
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14813 )
Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................
Patch Set 7: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 7
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Dec 2019 01:19:47 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9162: Do not apply inferred predicate to outer joins
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/14813 )
Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14813/6/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
File fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java:
http://gerrit.cloudera.org:8080/#/c/14813/6/fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java@1201
PS6, Line 1201: analyzer.markConjunctsAssigned(preds);
> I should note that I tried adding additional check in Analyzer.createEquivC
So, put differently, if we check for the outer join at the Subquery level (or WITH clause level), it doesn't know that the column came from an outer join. We could probably pass the inlineViewRef's expr substitution map down to that method and before creating the inferred predicate, first check the substitution map...but when I look at the Expr.trySubstitute() method, its more complicated than just a lookup .. it does a recursive substitution and also removes implicit casts.
--
To view, visit http://gerrit.cloudera.org:8080/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 7
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Dec 2019 01:23:30 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9162: Do not apply inferred predicate to outer joins
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/14813 )
Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................
IMPALA-9162: Do not apply inferred predicate to outer joins
When the planner migrates predicates to inline views, it also creates
equivalent predicates based on the value transfer graph which is built
by transitive relationships among join conditions. These newly inferred
predicates are placed typically as 'other predicates' of an inner or
outer join.
However, for outer joins, this has the effect of adding extra predicates
in the WHERE clause which is incorrect since it may filter NULL values.
Since the original query did not have null filtering conditions in
the WHERE clause, we should not add new ones. In this fix we do the
following: during the migration of conjuncts to inline views, analyze
the predicate of type A <op> B and if it is an inferred predicate AND
either the left or right slots reference the output tuple of an outer
join, the inferred predicate is ignored.
Note that simple queries with combination of inner and outer joins may
not reproduce the problem. Due to the nature of predicate inferencing,
some combination of subqueries, inner joins, outer joins is needed. For
the query pattern, please see the example in the JIRA.
Tests:
- Added plan tests with left and right outer joins to inline-view.test
- One baseline plan in inline-view.test had to be updated
- Manually ran few queries on impala shell to verify result
correctness: by checking that NULL values are being produced for outer
joins.
- Ran regression tests on jenkins
Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Reviewed-on: http://gerrit.cloudera.org:8080/14813
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M testdata/workloads/functional-planner/queries/PlannerTest/inline-view.test
2 files changed, 130 insertions(+), 1 deletion(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 8
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-9162: Do not apply inferred predicate to outer joins
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14813 )
Change subject: IMPALA-9162: Do not apply inferred predicate to outer joins
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5220/ : 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/14813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie9521bd768c4b333069c34d5c1e11b10ea535827
Gerrit-Change-Number: 14813
Gerrit-PatchSet: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.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: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 06 Dec 2019 03:05:02 +0000
Gerrit-HasComments: No