Re: Assert failure of the cross-check for nullingrels

Richard Guo <guofenglinux@gmail.com> writes:
> I tried with v4 patch and find that, as you predicted, it might reject
> all the clones in some cases.  Check the query below
> ...
> So the qual 't3.a = t4.a' is missing in this plan shape.

I poked into that more closely and realized that the reason that
clause_is_computable_at() misbehaves is that both clones of the
"t3.a = t4.a" qual have the same clause_relids: (4 5 6) which is
t3, the left join to t3, and t4.  This is unsurprising because
the difference in these clones is whether they are expected to be
evaluated above or below outer join 3 (the left join to t2), and
t2 doesn't appear in the qual.  (It does appear in "t2.b = t4.b",
which is why there's no similar misbehavior for that qual.)

If they have the same clause_relids, then clearly in its current
form clause_is_computable_at() cannot distinguish them.  So what
I now think we should do is have clause_is_computable_at() examine
their required_relids instead.  Those will be different, by
construction in deconstruct_distribute_oj_quals(), ensuring that
we select only one of the group of clones.

BTW, while I've not tried it, I suspect your v2 patch also fails
on this example for the same reason: it cannot distinguish the
clones of this qual.

            regards, tom lane

diff --git a/src/backend/optimizer/util/restrictinfo.c b/src/backend/optimizer/util/restrictinfo.c
index d2bc096e1c..760d24bebf 100644
--- a/src/backend/optimizer/util/restrictinfo.c
+++ b/src/backend/optimizer/util/restrictinfo.c
@@ -544,13 +544,24 @@ clause_is_computable_at(PlannerInfo *root,
                         RestrictInfo *rinfo,
                         Relids eval_relids)
 {
-    Relids        clause_relids = rinfo->clause_relids;
+    Relids        clause_relids;
     ListCell   *lc;

     /* Nothing to do if no outer joins have been performed yet. */
     if (!bms_overlap(eval_relids, root->outer_join_rels))
         return true;

+    /*
+     * For an ordinary qual clause, we consider the actual clause_relids as
+     * explained above.  However, it's possible for multiple members of a
+     * group of clone quals to have the same clause_relids, so for clones use
+     * the required_relids instead to ensure we select just one of them.
+     */
+    if (rinfo->has_clone || rinfo->is_clone)
+        clause_relids = rinfo->required_relids;
+    else
+        clause_relids = rinfo->clause_relids;
+
     foreach(lc, root->join_info_list)
     {
         SpecialJoinInfo *sjinfo = (SpecialJoinInfo *) lfirst(lc);
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 8fa2b376f3..1fd75c8f58 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -2500,6 +2500,74 @@ 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)
+
+explain (costs off)
+select * from int4_tbl t1
+  left join ((select t2.f1 from int4_tbl t2
+                left join int4_tbl t3 on t2.f1 > 0
+                where t3.f1 is null) s
+             left join tenk1 t4 on s.f1 > 1)
+    on s.f1 = t1.f1;
+                   QUERY PLAN
+-------------------------------------------------
+ Nested Loop Left Join
+   Join Filter: (t2.f1 > 1)
+   ->  Hash Right Join
+         Hash Cond: (t2.f1 = t1.f1)
+         ->  Nested Loop Left Join
+               Join Filter: (t2.f1 > 0)
+               Filter: (t3.f1 IS NULL)
+               ->  Seq Scan on int4_tbl t2
+               ->  Materialize
+                     ->  Seq Scan on int4_tbl t3
+         ->  Hash
+               ->  Seq Scan on int4_tbl t1
+   ->  Seq Scan on tenk1 t4
+(13 rows)
+
+explain (costs off)
+select * from onek t1
+    left join onek t2 on t1.unique1 = t2.unique1
+    left join onek t3 on t2.unique1 = t3.unique1
+    left join onek t4 on t3.unique1 = t4.unique1 and t2.unique2 = t4.unique2;
+                               QUERY PLAN
+------------------------------------------------------------------------
+ Hash Left Join
+   Hash Cond: ((t3.unique1 = t4.unique1) AND (t2.unique2 = t4.unique2))
+   ->  Hash Left Join
+         Hash Cond: (t2.unique1 = t3.unique1)
+         ->  Hash Left Join
+               Hash Cond: (t1.unique1 = t2.unique1)
+               ->  Seq Scan on onek t1
+               ->  Hash
+                     ->  Seq Scan on onek t2
+         ->  Hash
+               ->  Seq Scan on onek t3
+   ->  Hash
+         ->  Seq Scan on onek 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 641fd1a21b..84547c7dff 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -488,6 +488,26 @@ 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);
+
+explain (costs off)
+select * from int4_tbl t1
+  left join ((select t2.f1 from int4_tbl t2
+                left join int4_tbl t3 on t2.f1 > 0
+                where t3.f1 is null) s
+             left join tenk1 t4 on s.f1 > 1)
+    on s.f1 = t1.f1;
+
+explain (costs off)
+select * from onek t1
+    left join onek t2 on t1.unique1 = t2.unique1
+    left join onek t3 on t2.unique1 = t3.unique1
+    left join onek t4 on t3.unique1 = t4.unique1 and t2.unique2 = t4.unique2;
+
 --
 -- check a case where we formerly got confused by conflicting sort orders
 -- in redundant merge join path keys