You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Alex Behm (Code Review)" <ge...@cloudera.org> on 2017/09/12 16:13:49 UTC
[Impala-ASF-CR] IMPALA-5856: Fix outer join predicate assignment.
Alex Behm has uploaded a new change for review.
http://gerrit.cloudera.org:8080/8039
Change subject: IMPALA-5856: Fix outer join predicate assignment.
......................................................................
IMPALA-5856: Fix outer join predicate assignment.
Fixes incorrect assignment of join predicates with
the following properties:
- from the On-clause of a left outer join
- only references the left-hand side tuples (not the
right hand side tuple)
- references full-outer joined tuples; the full outer
join appears on the left
Testing:
- a core/hdfs run passed
- added new regression test
Change-Id: I93db34d988cb66e00aa05d7dc161e0ca47042acb
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M testdata/workloads/functional-planner/queries/PlannerTest/outer-joins.test
2 files changed, 43 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/39/8039/1
--
To view, visit http://gerrit.cloudera.org:8080/8039
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I93db34d988cb66e00aa05d7dc161e0ca47042acb
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
[Impala-ASF-CR] IMPALA-5856: Fix outer join predicate assignment.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-5856: Fix outer join predicate assignment.
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1222/
--
To view, visit http://gerrit.cloudera.org:8080/8039
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I93db34d988cb66e00aa05d7dc161e0ca47042acb
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5856: Fix outer join predicate assignment.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-5856: Fix outer join predicate assignment.
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/8039
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I93db34d988cb66e00aa05d7dc161e0ca47042acb
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5856: Fix outer join predicate assignment.
Posted by "Dimitris Tsirogiannis (Code Review)" <ge...@cloudera.org>.
Dimitris Tsirogiannis has posted comments on this change.
Change subject: IMPALA-5856: Fix outer join predicate assignment.
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/8039
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I93db34d988cb66e00aa05d7dc161e0ca47042acb
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5856: Fix outer join predicate assignment.
Posted by "Dimitris Tsirogiannis (Code Review)" <ge...@cloudera.org>.
Dimitris Tsirogiannis has posted comments on this change.
Change subject: IMPALA-5856: Fix outer join predicate assignment.
......................................................................
Patch Set 1:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/8039/1/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
PS1, Line 1381: // Right-most full-outer join table ref that is equal to or to the left of the
: // table ref materializing 'tids'.
Trying to visualize this hurts my brain :) Not sure if it helps to understand the logic in this function. The comment in L1385 should suffice.
PS1, Line 1385: this
What is "this" refer to? After reading this more carefully, I think I understand what it means. I think it is better to move the comment below L1388.
PS1, Line 1398: globalState_.ojClauseByConjunct.get(e.getId());
getOjRef(e)?
http://gerrit.cloudera.org:8080/#/c/8039/1/testdata/workloads/functional-planner/queries/PlannerTest/outer-joins.test
File testdata/workloads/functional-planner/queries/PlannerTest/outer-joins.test:
PS1, Line 1020: from the On-clause of a left outer join
Out of curiosity, did you check if we have the same issue if there is a left outer join followed by a full outer join? If we don't have a test like that, maybe add it for completion.
Line 1028: and t1.bigint_col > 10 and t2.bigint_col > 30
To make it more interesting, add a where clause with one predicate that references t1 and t2 and one that references t3.
--
To view, visit http://gerrit.cloudera.org:8080/8039
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I93db34d988cb66e00aa05d7dc161e0ca47042acb
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5856: Fix outer join predicate assignment.
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: IMPALA-5856: Fix outer join predicate assignment.
......................................................................
Patch Set 1:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/8039/1/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
PS1, Line 1381: // Right-most full-outer join table ref that is equal to or to the left of the
: // table ref materializing 'tids'.
> Trying to visualize this hurts my brain :) Not sure if it helps to understa
Agree, difficult to comprehend. Removed.
PS1, Line 1385: this
> What is "this" refer to? After reading this more carefully, I think I under
Rephrased. The comment is supposed to describe what 'ojRef' is - that's why it's above 'ojRef'.
PS1, Line 1398: globalState_.ojClauseByConjunct.get(e.getId());
> getOjRef(e)?
Done
http://gerrit.cloudera.org:8080/#/c/8039/1/testdata/workloads/functional-planner/queries/PlannerTest/outer-joins.test
File testdata/workloads/functional-planner/queries/PlannerTest/outer-joins.test:
PS1, Line 1020: from the On-clause of a left outer join
> Out of curiosity, did you check if we have the same issue if there is a lef
We have a test for that in L634. Please check if you think it's sufficient.
I don't think those cases are similar because if the FOJ comes after the LOJ, then only the FOJ On-clause predicates are considered to be full-outer-joined (and will be handled by the non-full-outer join assignment code path).
Line 1028: and t1.bigint_col > 10 and t2.bigint_col > 30
> To make it more interesting, add a where clause with one predicate that ref
Good idea. Done.
--
To view, visit http://gerrit.cloudera.org:8080/8039
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I93db34d988cb66e00aa05d7dc161e0ca47042acb
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5856: Fix outer join predicate assignment.
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: IMPALA-5856: Fix outer join predicate assignment.
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/8039
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I93db34d988cb66e00aa05d7dc161e0ca47042acb
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5856: Fix outer join predicate assignment.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged.
Change subject: IMPALA-5856: Fix outer join predicate assignment.
......................................................................
IMPALA-5856: Fix outer join predicate assignment.
Fixes incorrect assignment of join predicates with
the following properties:
- from the On-clause of a left outer join
- only references the left-hand side tuples (not the
right hand side tuple)
- references full-outer joined tuples; the full outer
join appears on the left
Testing:
- a core/hdfs run passed
- added new regression test
Change-Id: I93db34d988cb66e00aa05d7dc161e0ca47042acb
Reviewed-on: http://gerrit.cloudera.org:8080/8039
Reviewed-by: Alex Behm <al...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M testdata/workloads/functional-planner/queries/PlannerTest/outer-joins.test
2 files changed, 46 insertions(+), 5 deletions(-)
Approvals:
Impala Public Jenkins: Verified
Alex Behm: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/8039
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I93db34d988cb66e00aa05d7dc161e0ca47042acb
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-5856: Fix outer join predicate assignment.
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has uploaded a new patch set (#2).
Change subject: IMPALA-5856: Fix outer join predicate assignment.
......................................................................
IMPALA-5856: Fix outer join predicate assignment.
Fixes incorrect assignment of join predicates with
the following properties:
- from the On-clause of a left outer join
- only references the left-hand side tuples (not the
right hand side tuple)
- references full-outer joined tuples; the full outer
join appears on the left
Testing:
- a core/hdfs run passed
- added new regression test
Change-Id: I93db34d988cb66e00aa05d7dc161e0ca47042acb
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M testdata/workloads/functional-planner/queries/PlannerTest/outer-joins.test
2 files changed, 46 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/39/8039/2
--
To view, visit http://gerrit.cloudera.org:8080/8039
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I93db34d988cb66e00aa05d7dc161e0ca47042acb
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>