Repository: incubator-impala Updated Branches: refs/heads/master 082058e2c -> fcf32b584
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 <[email protected]> Tested-by: Impala Public Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/3e770405 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/3e770405 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/3e770405 Branch: refs/heads/master Commit: 3e770405e328f883610ce40867f770e588839c13 Parents: 082058e Author: Alex Behm <[email protected]> Authored: Fri Aug 18 10:36:13 2017 -0700 Committer: Impala Public Jenkins <[email protected]> Committed: Sat Aug 19 20:56:10 2017 +0000 ---------------------------------------------------------------------- be/src/exprs/tuple-is-null-predicate.cc | 4 +++- be/src/exprs/tuple-is-null-predicate.h | 7 +++++-- .../queries/QueryTest/outer-joins.test | 16 ++++++++++++++++ 3 files changed, 24 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/3e770405/be/src/exprs/tuple-is-null-predicate.cc ---------------------------------------------------------------------- diff --git a/be/src/exprs/tuple-is-null-predicate.cc b/be/src/exprs/tuple-is-null-predicate.cc index 2762247..bb532b2 100644 --- a/be/src/exprs/tuple-is-null-predicate.cc +++ b/be/src/exprs/tuple-is-null-predicate.cc @@ -33,7 +33,9 @@ BooleanVal TupleIsNullPredicate::GetBooleanVal( for (int i = 0; i < tuple_idxs_.size(); ++i) { count += row->GetTuple(tuple_idxs_[i]) == NULL; } - return BooleanVal(!tuple_idxs_.empty() && count == tuple_idxs_.size()); + // Return true only if all originally specified tuples are NULL. Return false if any + // tuple is non-nullable. + return BooleanVal(count == tuple_ids_.size()); } TupleIsNullPredicate::TupleIsNullPredicate(const TExprNode& node) http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/3e770405/be/src/exprs/tuple-is-null-predicate.h ---------------------------------------------------------------------- diff --git a/be/src/exprs/tuple-is-null-predicate.h b/be/src/exprs/tuple-is-null-predicate.h index 611f7e6..ea631c0 100644 --- a/be/src/exprs/tuple-is-null-predicate.h +++ b/be/src/exprs/tuple-is-null-predicate.h @@ -26,9 +26,12 @@ namespace impala { class TExprNode; /// Returns true if all of the given tuples are NULL, false otherwise. +/// Returns false if any of the given tuples is non-nullable. /// It is important that this predicate not require the given tuples to be nullable, -/// because the FE may sometimes wrap expressions in this predicate that contain SlotRefs -/// on non-nullable tuples (see IMPALA-904). +/// because the FE sometimes wrap expressions in this predicate that contain SlotRefs +/// on non-nullable tuples (see IMPALA-904/IMPALA-5504). This happens for exprs that +/// are evaluated before the outer join that makes the tuples given to this predicate +/// nullable, e.g., in the ON-clause of that join. /// TODO: Implement codegen to eliminate overhead on non-nullable tuples. class TupleIsNullPredicate: public Predicate { protected: http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/3e770405/testdata/workloads/functional-query/queries/QueryTest/outer-joins.test ---------------------------------------------------------------------- diff --git a/testdata/workloads/functional-query/queries/QueryTest/outer-joins.test b/testdata/workloads/functional-query/queries/QueryTest/outer-joins.test index 72b0a7a..edb1e84 100644 --- a/testdata/workloads/functional-query/queries/QueryTest/outer-joins.test +++ b/testdata/workloads/functional-query/queries/QueryTest/outer-joins.test @@ -712,3 +712,19 @@ ON t1.id = v3.id ---- TYPES int,bigint,int,bigint ==== +---- QUERY +# IMPALA-5504: Check that a TupleIsNullPredicate evaluates to false if one of tuples to +# check is non-nullable. This happens for exprs that are evaluated before the outer join +# that makes those tuples nullable, e.g., in the ON-clause of that join. +SELECT /* +straight_join */ COUNT(t1.id) +FROM functional.alltypessmall t1 +LEFT OUTER JOIN ( + SELECT /* +straight_join */ IFNULL(t2.int_col, 1) AS c + FROM functional.alltypessmall t2 + LEFT OUTER JOIN functional.alltypestiny t3 ON t2.id = t3.id + 1000 +) v ON t1.int_col = v.c; +---- RESULTS +1040 +---- TYPES +bigint +====
