Fix two separate bugs in setrefs.c. set_subqueryscan_references needs

to copy the whole plan tree before invoking adjust_plan_varnos(); else
if there is any multiply-linked substructure, the latter might increment
some Var's varno twice.  Previously there were some retail copyObject
calls inside adjust_plan_varnos, but it seems a lot safer to just dup the
whole tree first.  Also, set_inner_join_references was trying to avoid
work by not recursing if a BitmapHeapScan's bitmapqualorig contained no
outer references; which was OK at the time the code was written, I think,
but now that create_bitmap_scan_plan removes duplicate clauses from
bitmapqualorig it is possible for that field to be NULL while outer
references still remain in the qpqual and/or contained indexscan nodes.
For safety, always recurse even if the BitmapHeapScan looks to be outer
reference free.  Per reports from Michael Fuhr and Oleg Bartunov.

diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 5f6e8301fc326a781e9f673f39dad33a3748ebe7..a07bae8b8421377dabea1ca5910de219e07e3a78 100644 (file)
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/optimizer/plan/setrefs.c,v 1.111 2005/06/10 00:28:54 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/optimizer/plan/setrefs.c,v 1.112 2005/08/27 18:04:49 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -367,7 +367,12 @@ set_subqueryscan_references(SubqueryScan *plan, List *rtable)
                                                   QTW_IGNORE_RT_SUBQUERIES);
                rtable = list_concat(rtable, sub_rtable);
 
-               result = plan->subplan;
+               /*
+                * we have to copy the subplan to make sure there are no duplicately
+                * linked nodes in it, else adjust_plan_varnos might increment some
+                * varnos twice
+                */
+               result = copyObject(plan->subplan);
 
                adjust_plan_varnos(result, rtoffset);
 
@@ -538,10 +543,7 @@ adjust_plan_varnos(Plan *plan, int rtoffset)
                         * Even though the targetlist won't be used by the executor,
                         * we fix it up for possible use by EXPLAIN (not to mention
                         * ease of debugging --- wrong varnos are very confusing).
-                        * We have to make a copy because the tlist is very likely
-                        * shared with lower plan levels.
                         */
-                       plan->targetlist = copyObject(plan->targetlist);
                        adjust_expr_varnos((Node *) plan->targetlist, rtoffset);
                        Assert(plan->qual == NIL);
                        break;
@@ -552,7 +554,6 @@ adjust_plan_varnos(Plan *plan, int rtoffset)
                         * or quals.  It does have live expressions for limit/offset,
                         * however.
                         */
-                       plan->targetlist = copyObject(plan->targetlist);
                        adjust_expr_varnos((Node *) plan->targetlist, rtoffset);
                        Assert(plan->qual == NIL);
                        adjust_expr_varnos(((Limit *) plan)->limitOffset, rtoffset);
@@ -569,14 +570,6 @@ adjust_plan_varnos(Plan *plan, int rtoffset)
                        adjust_expr_varnos(((Result *) plan)->resconstantqual, rtoffset);
                        break;
                case T_Append:
-
-                       /*
-                        * Append, like Sort et al, doesn't actually evaluate its
-                        * targetlist or check quals, and we haven't bothered to give it
-                        * its own tlist copy. So, copy tlist before fixing.  Then
-                        * recurse into child plans.
-                        */
-                       plan->targetlist = copyObject(plan->targetlist);
                        adjust_expr_varnos((Node *) plan->targetlist, rtoffset);
                        Assert(plan->qual == NIL);
                        foreach(l, ((Append *) plan)->appendplans)
@@ -849,39 +842,38 @@ set_inner_join_references(Plan *inner_plan,
                /*
                 * The inner side is a bitmap scan plan.  Fix the top node,
                 * and recurse to get the lower nodes.
+                *
+                * Note: create_bitmap_scan_plan removes clauses from bitmapqualorig
+                * if they are duplicated in qpqual, so must test these independently.
                 */
                BitmapHeapScan *innerscan = (BitmapHeapScan *) inner_plan;
+               Index           innerrel = innerscan->scan.scanrelid;
                List       *bitmapqualorig = innerscan->bitmapqualorig;
 
-               /* No work needed if bitmapqual refers only to its own rel... */
+               /* only refs to outer vars get changed in the inner qual */
                if (NumRelids((Node *) bitmapqualorig) > 1)
-               {
-                       Index           innerrel = innerscan->scan.scanrelid;
-
-                       /* only refs to outer vars get changed in the inner qual */
                        innerscan->bitmapqualorig = join_references(bitmapqualorig,
-                                                                                                         rtable,
-                                                                                                         outer_itlist,
-                                                                                                         NULL,
-                                                                                                         innerrel);
-
-                       /*
-                        * We must fix the inner qpqual too, if it has join
-                        * clauses (this could happen if special operators are
-                        * involved: some indexquals may get rechecked as qpquals).
-                        */
-                       if (NumRelids((Node *) inner_plan->qual) > 1)
-                               inner_plan->qual = join_references(inner_plan->qual,
-                                                                                                  rtable,
-                                                                                                  outer_itlist,
-                                                                                                  NULL,
-                                                                                                  innerrel);
+                                                                                                               rtable,
+                                                                                                               outer_itlist,
+                                                                                                               NULL,
+                                                                                                               innerrel);
 
-                       /* Now recurse */
-                       set_inner_join_references(inner_plan->lefttree,
-                                                                         rtable,
-                                                                         outer_itlist);
-               }
+               /*
+                * We must fix the inner qpqual too, if it has join
+                * clauses (this could happen if special operators are
+                * involved: some indexquals may get rechecked as qpquals).
+                */
+               if (NumRelids((Node *) inner_plan->qual) > 1)
+                       inner_plan->qual = join_references(inner_plan->qual,
+                                                                                          rtable,
+                                                                                          outer_itlist,
+                                                                                          NULL,
+                                                                                          innerrel);
+
+               /* Now recurse */
+               set_inner_join_references(inner_plan->lefttree,
+                                                                 rtable,
+                                                                 outer_itlist);
        }
        else if (IsA(inner_plan, BitmapAnd))
        {