From 12cc5081783e435bbd2e577e8f7666c1ebe7d28a Mon Sep 17 00:00:00 2001
From: Alex Behm <alex.behm@cloudera.com>
Date: Mon, 7 Nov 2016 17:32:57 -0800
Subject: [PATCH] IMPALA-3125: Fix assignment of equality predicates from an
 outer-join On-clause.

Impala used to incorrectly assign On-clause equality predicates from an
outer join if those predicates referenced multiple tables, but only one
side of the outer join.

The fix is to add an additional check in Analyzer.getEqJoinConjuncts()
to prevent that incorrect assignment.

Change-Id: I719e0eeacccad070b1f9509d80aaf761b572add0
Reviewed-on: http://gerrit.cloudera.org:8080/4986
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Internal Jenkins
---
 .../org/apache/impala/analysis/Analyzer.java  | 18 ++++++++++--
 .../queries/PlannerTest/outer-joins.test      | 29 +++++++++++++++++++
 2 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/fe/src/main/java/org/apache/impala/analysis/Analyzer.java b/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
index 96ab09772..48193426d 100644
--- a/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
+++ b/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
@@ -1316,9 +1316,11 @@ public class Analyzer {
       Expr e = globalState_.conjuncts.get(conjunctId);
       Preconditions.checkState(e != null);
       if (!canEvalFullOuterJoinedConjunct(e, nodeTblRefIds) ||
-          !canEvalAntiJoinedConjunct(e, nodeTblRefIds)) {
+          !canEvalAntiJoinedConjunct(e, nodeTblRefIds) ||
+          !canEvalOuterJoinedConjunct(e, nodeTblRefIds)) {
         continue;
       }
+
       if (ojClauseConjuncts != null && !ojClauseConjuncts.contains(conjunctId)) continue;
       result.add(e);
     }
@@ -1326,8 +1328,8 @@ public class Analyzer {
   }
 
   /**
-   * Checks if a conjunct can be evaluated at a node materializing a list of tuple ids
-   * 'tids'.
+   * Returns false if 'e' references a full outer joined tuple and it is incorrect to
+   * evaluate 'e' at a node materializing 'tids'. Returns true otherwise.
    */
   public boolean canEvalFullOuterJoinedConjunct(Expr e, List<TupleId> tids) {
     TableRef fullOuterJoin = getFullOuterJoinRef(e);
@@ -1335,6 +1337,16 @@ public class Analyzer {
     return tids.containsAll(fullOuterJoin.getAllTableRefIds());
   }
 
+  /**
+   * Returns false if 'e' originates from an outer-join On-clause and it is incorrect to
+   * evaluate 'e' at a node materializing 'tids'. Returns true otherwise.
+   */
+  public boolean canEvalOuterJoinedConjunct(Expr e, List<TupleId> tids) {
+    TableRef outerJoin = globalState_.ojClauseByConjunct.get(e.getId());
+    if (outerJoin == null) return true;
+    return tids.containsAll(outerJoin.getAllTableRefIds());
+  }
+
   /**
    * Returns true if predicate 'e' can be correctly evaluated by a tree materializing
    * 'tupleIds', otherwise false:
diff --git a/testdata/workloads/functional-planner/queries/PlannerTest/outer-joins.test b/testdata/workloads/functional-planner/queries/PlannerTest/outer-joins.test
index 3a39d14c5..2d5d6cd35 100644
--- a/testdata/workloads/functional-planner/queries/PlannerTest/outer-joins.test
+++ b/testdata/workloads/functional-planner/queries/PlannerTest/outer-joins.test
@@ -890,3 +890,32 @@ PLAN-ROOT SINK
 04:SCAN HDFS [functional.alltypestiny e]
    partitions=4/4 files=4 size=460B
 ====
+# IMPALA-3125: Test that the On-clause predicates from an outer join are assigned to the
+# corresponding outer-join node, even if the predicates do not reference the join rhs.
+select a.id aid, b.id bid, a.int_col aint, b.int_col bint
+from functional.alltypes a
+inner join functional.alltypes b
+  on a.int_col = b.int_col
+left outer join functional.alltypes c
+  on a.id = b.id and b.bigint_col = c.bigint_col
+---- PLAN
+PLAN-ROOT SINK
+|
+04:HASH JOIN [LEFT OUTER JOIN]
+|  hash predicates: b.bigint_col = c.bigint_col
+|  other join predicates: a.id = b.id
+|
+|--02:SCAN HDFS [functional.alltypes c]
+|     partitions=24/24 files=24 size=478.45KB
+|
+03:HASH JOIN [INNER JOIN]
+|  hash predicates: b.int_col = a.int_col
+|  runtime filters: RF000 <- a.int_col
+|
+|--00:SCAN HDFS [functional.alltypes a]
+|     partitions=24/24 files=24 size=478.45KB
+|
+01:SCAN HDFS [functional.alltypes b]
+   partitions=24/24 files=24 size=478.45KB
+   runtime filters: RF000 -> b.int_col
+====