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/08/19 05:10:07 UTC
[Impala-ASF-CR] IMPALA-5504: Fix TupleIsNullPredicate evaluation.
Alex Behm has uploaded a new change for review.
http://gerrit.cloudera.org:8080/7737
Change subject: IMPALA-5504: Fix TupleIsNullPredicate evaluation.
......................................................................
IMPALA-5504: Fix TupleIsNullPredicate evaluation.
There was a bug in the BE evaluation logic of the
TupleIsNullPredicate which could lead to wrong results
for certain plan shapes.
A TupleIsNullPredicate should evaluate to true only if
all specified tuples are NULL. This was always the intent
of the FE and is also documented in the BE as the required
behavior.
Testing:
- Added regression test
- Core tests passed
Change-Id: Id659f849a68d88cfe22c65dd1747dd6d6a916163
---
M be/src/exprs/tuple-is-null-predicate.cc
M be/src/exprs/tuple-is-null-predicate.h
M testdata/workloads/functional-query/queries/QueryTest/outer-joins.test
3 files changed, 24 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/7737/1
--
To view, visit http://gerrit.cloudera.org:8080/7737
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id659f849a68d88cfe22c65dd1747dd6d6a916163
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
[Impala-ASF-CR] IMPALA-5504: Fix TupleIsNullPredicate evaluation.
Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.
Change subject: IMPALA-5504: Fix TupleIsNullPredicate evaluation.
......................................................................
Patch Set 1: Code-Review+2
Nice catch
--
To view, visit http://gerrit.cloudera.org:8080/7737
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id659f849a68d88cfe22c65dd1747dd6d6a916163
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5504: Fix TupleIsNullPredicate evaluation.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged.
Change subject: IMPALA-5504: Fix TupleIsNullPredicate evaluation.
......................................................................
IMPALA-5504: Fix TupleIsNullPredicate evaluation.
There was a bug in the BE evaluation logic of the
TupleIsNullPredicate which could lead to wrong results
for certain plan shapes.
A TupleIsNullPredicate should evaluate to true only if
all specified tuples are NULL. This was always the intent
of the FE and is also documented in the BE as the required
behavior.
Testing:
- Added regression test
- Core tests passed
Change-Id: Id659f849a68d88cfe22c65dd1747dd6d6a916163
Reviewed-on: http://gerrit.cloudera.org:8080/7737
Reviewed-by: Matthew Jacobs <mj...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M be/src/exprs/tuple-is-null-predicate.cc
M be/src/exprs/tuple-is-null-predicate.h
M testdata/workloads/functional-query/queries/QueryTest/outer-joins.test
3 files changed, 24 insertions(+), 3 deletions(-)
Approvals:
Impala Public Jenkins: Verified
Matthew Jacobs: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/7737
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Id659f849a68d88cfe22c65dd1747dd6d6a916163
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-5504: Fix TupleIsNullPredicate evaluation.
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-5504: Fix TupleIsNullPredicate evaluation.
......................................................................
Patch Set 1:
Looking at git blame, http://gerrit.cloudera.org/445 did the reverse of this to fix IMPALA-1987. I'm guessing the regression test passed but it seems like it's worth understanding why the fix was wrong and not needed.
--
To view, visit http://gerrit.cloudera.org:8080/7737
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id659f849a68d88cfe22c65dd1747dd6d6a916163
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5504: Fix TupleIsNullPredicate evaluation.
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-5504: Fix TupleIsNullPredicate evaluation.
......................................................................
Patch Set 2:
Thanks for the explanation!
--
To view, visit http://gerrit.cloudera.org:8080/7737
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id659f849a68d88cfe22c65dd1747dd6d6a916163
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: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5504: Fix TupleIsNullPredicate evaluation.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-5504: Fix TupleIsNullPredicate evaluation.
......................................................................
Patch Set 1: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/7737
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id659f849a68d88cfe22c65dd1747dd6d6a916163
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5504: Fix TupleIsNullPredicate evaluation.
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: IMPALA-5504: Fix TupleIsNullPredicate evaluation.
......................................................................
Patch Set 2:
Tim has a good point and I did investigate that.
The underlying problem in this JIRA and in IMPALA-1987 is the same, but IMPALA-1987 did not fix is correctly.
The code before the fix for IMPALA-1987 was not correct because it only checked the nullable tuples in that loop. But the predicate should always return false if there is at least one non-nullable tuple.
The fix IMPALA-1987 for was still not correct for the reasons stated in the commit message here.
--
To view, visit http://gerrit.cloudera.org:8080/7737
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id659f849a68d88cfe22c65dd1747dd6d6a916163
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: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5504: Fix TupleIsNullPredicate evaluation.
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.
Change subject: IMPALA-5504: Fix TupleIsNullPredicate evaluation.
......................................................................
Patch Set 1:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1100/
--
To view, visit http://gerrit.cloudera.org:8080/7737
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id659f849a68d88cfe22c65dd1747dd6d6a916163
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: No