Re: Assert failure of the cross-check for nullingrels

Richard Guo <guofenglinux@gmail.com> writes:
> Here is an updated patch with comments and test case.  I also change the
> code to store 'group_clause_relids' directly in RestrictInfo.

Hmm ... I don't like this patch terribly much.  It's not obvious why
(or if) it works, and it's squirreling bits of semantic knowledge
into places they don't belong.  ISTM the fundamental problem is that
clause_is_computable_at() is accepting clauses it shouldn't, and we
should try to solve it there.

After some poking at it I hit on what seems like a really simple
solution: we should be checking syn_righthand not min_righthand
to see whether a Var should be considered nullable by a given OJ.
Maybe that's still not quite right, but it seems like it might be
right given that the last fix reaffirmed our conviction that Vars
should be marked according to the syntactic structure.

If we don't want to do it like this, another way is to consider
the transitive closure of commutable outer joins, along similar
lines to your fixes to my earlier patch.  But that seems like it
might just be adding complication.

            regards, tom lane

diff --git a/src/backend/optimizer/util/restrictinfo.c b/src/backend/optimizer/util/restrictinfo.c
index c44bd2f815..23ab4177fa 100644
--- a/src/backend/optimizer/util/restrictinfo.c
+++ b/src/backend/optimizer/util/restrictinfo.c
@@ -563,9 +563,9 @@ clause_is_computable_at(PlannerInfo *root,
             continue;

         /* Else, trouble if clause mentions any nullable Vars. */
-        if (bms_overlap(clause_relids, sjinfo->min_righthand) ||
+        if (bms_overlap(clause_relids, sjinfo->syn_righthand) ||
             (sjinfo->jointype == JOIN_FULL &&
-             bms_overlap(clause_relids, sjinfo->min_lefthand)))
+             bms_overlap(clause_relids, sjinfo->syn_lefthand)))
             return false;        /* doesn't work */
     }

diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 9bafadde66..af68567561 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -2500,6 +2500,28 @@ select * from int4_tbl t1
                      ->  Seq Scan on int4_tbl t4
 (12 rows)

+explain (costs off)
+select * from int4_tbl t1
+  left join (int4_tbl t2 left join int4_tbl t3 on t2.f1 > 0) on t2.f1 > 1
+  left join int4_tbl t4 on t2.f1 > 2 and t3.f1 > 3
+where t1.f1 = coalesce(t2.f1, 1);
+                     QUERY PLAN
+----------------------------------------------------
+ Nested Loop Left Join
+   Join Filter: ((t2.f1 > 2) AND (t3.f1 > 3))
+   ->  Nested Loop Left Join
+         Join Filter: (t2.f1 > 0)
+         ->  Nested Loop Left Join
+               Filter: (t1.f1 = COALESCE(t2.f1, 1))
+               ->  Seq Scan on int4_tbl t1
+               ->  Materialize
+                     ->  Seq Scan on int4_tbl t2
+                           Filter: (f1 > 1)
+         ->  Seq Scan on int4_tbl t3
+   ->  Materialize
+         ->  Seq Scan on int4_tbl t4
+(13 rows)
+
 --
 -- check a case where we formerly got confused by conflicting sort orders
 -- in redundant merge join path keys
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index a44234b0af..c7a39abd23 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -488,6 +488,12 @@ select * from int4_tbl t1
   left join int4_tbl t3 on t2.f1 = t3.f1
   left join int4_tbl t4 on t3.f1 != t4.f1;

+explain (costs off)
+select * from int4_tbl t1
+  left join (int4_tbl t2 left join int4_tbl t3 on t2.f1 > 0) on t2.f1 > 1
+  left join int4_tbl t4 on t2.f1 > 2 and t3.f1 > 3
+where t1.f1 = coalesce(t2.f1, 1);
+
 --
 -- check a case where we formerly got confused by conflicting sort orders
 -- in redundant merge join path keys