Repair incorrect placement of WHERE clauses when there are multiple,

rearrangeable outer joins and the WHERE clause is non-strict and mentions
only nullable-side relations.  New bug in 8.2, caused by new logic to allow
rearranging outer joins.  Per bug #2807 from Ross Cohen; thanks to Jeff
Davis for producing a usable test case.

diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index 2a8e1f528e9857a7105e74f8515fed66a48f64cb..da321a637b21035c9682c33c0af632b6968aaccb 100644 (file)
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/optimizer/plan/initsplan.c,v 1.123 2006/10/04 00:29:54 momjian Exp $
+ *   $PostgreSQL: pgsql/src/backend/optimizer/plan/initsplan.c,v 1.124 2006/12/07 19:33:40 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -735,30 +735,69 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
         * For a non-outer-join qual, we can evaluate the qual as soon as (1)
         * we have all the rels it mentions, and (2) we are at or above any
         * outer joins that can null any of these rels and are below the
-        * syntactic location of the given qual. To enforce the latter, scan
-        * the oj_info_list and merge the required-relid sets of any such OJs
-        * into the clause's own reference list.  At the time we are called,
-        * the oj_info_list contains only outer joins below this qual.
+        * syntactic location of the given qual.  We must enforce (2) because
+        * pushing down such a clause below the OJ might cause the OJ to emit
+        * null-extended rows that should not have been formed, or that should
+        * have been rejected by the clause.  (This is only an issue for
+        * non-strict quals, since if we can prove a qual mentioning only
+        * nullable rels is strict, we'd have reduced the outer join to an
+        * inner join in reduce_outer_joins().)
+        *
+        * To enforce (2), scan the oj_info_list and merge the required-relid
+        * sets of any such OJs into the clause's own reference list.  At the
+        * time we are called, the oj_info_list contains only outer joins
+        * below this qual.  We have to repeat the scan until no new relids
+        * get added; this ensures that the qual is suitably delayed regardless
+        * of the order in which OJs get executed.  As an example, if we have
+        * one OJ with LHS=A, RHS=B, and one with LHS=B, RHS=C, it is implied
+        * that these can be done in either order; if the B/C join is done
+        * first then the join to A can null C, so a qual actually mentioning
+        * only C cannot be applied below the join to A.
         */
-       Relids      addrelids = NULL;
-       ListCell   *l;
+       bool        found_some;
 
        outerjoin_delayed = false;
-       foreach(l, root->oj_info_list)
-       {
-           OuterJoinInfo *ojinfo = (OuterJoinInfo *) lfirst(l);
+       do {
+           ListCell   *l;
 
-           if (bms_overlap(relids, ojinfo->min_righthand) ||
-               (ojinfo->is_full_join &&
-                bms_overlap(relids, ojinfo->min_lefthand)))
+           found_some = false;
+           foreach(l, root->oj_info_list)
            {
-               addrelids = bms_add_members(addrelids, ojinfo->min_lefthand);
-               addrelids = bms_add_members(addrelids, ojinfo->min_righthand);
-               outerjoin_delayed = true;
+               OuterJoinInfo *ojinfo = (OuterJoinInfo *) lfirst(l);
+
+               /* do we have any nullable rels of this OJ? */
+               if (bms_overlap(relids, ojinfo->min_righthand) ||
+                   (ojinfo->is_full_join &&
+                    bms_overlap(relids, ojinfo->min_lefthand)))
+               {
+                   /* yes; do we have all its rels? */
+                   if (!bms_is_subset(ojinfo->min_lefthand, relids) ||
+                       !bms_is_subset(ojinfo->min_righthand, relids))
+                   {
+                       /* no, so add them in */
+                       relids = bms_add_members(relids,
+                                                ojinfo->min_lefthand);
+                       relids = bms_add_members(relids,
+                                                ojinfo->min_righthand);
+                       outerjoin_delayed = true;
+                       /* we'll need another iteration */
+                       found_some = true;
+                   }
+               }
            }
-       }
+       } while (found_some);
 
-       if (bms_is_subset(addrelids, relids))
+       if (outerjoin_delayed)
+       {
+           /* Should still be a subset of current scope ... */
+           Assert(bms_is_subset(relids, qualscope));
+           /*
+            * Because application of the qual will be delayed by outer join,
+            * we mustn't assume its vars are equal everywhere.
+            */
+           maybe_equijoin = false;
+       }
+       else
        {
            /*
             * Qual is not delayed by any lower outer-join restriction. If it
@@ -774,19 +813,8 @@ distribute_qual_to_rels(PlannerInfo *root, Node *clause,
            else
                maybe_equijoin = false;
        }
-       else
-       {
-           relids = bms_union(relids, addrelids);
-           /* Should still be a subset of current scope ... */
-           Assert(bms_is_subset(relids, qualscope));
 
-           /*
-            * Because application of the qual will be delayed by outer join,
-            * we mustn't assume its vars are equal everywhere.
-            */
-           maybe_equijoin = false;
-       }
-       bms_free(addrelids);
+       /* Since it doesn't mention the LHS, it's certainly not an OJ clause */
        maybe_outer_join = false;
    }