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.
This commit is contained in:
Tom Lane 2005-08-27 18:04:49 +00:00
parent 5824d02155
commit 5a7d36973a

View File

@ -9,7 +9,7 @@
* *
* *
* IDENTIFICATION * 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); QTW_IGNORE_RT_SUBQUERIES);
rtable = list_concat(rtable, sub_rtable); 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); 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, * Even though the targetlist won't be used by the executor,
* we fix it up for possible use by EXPLAIN (not to mention * we fix it up for possible use by EXPLAIN (not to mention
* ease of debugging --- wrong varnos are very confusing). * 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); adjust_expr_varnos((Node *) plan->targetlist, rtoffset);
Assert(plan->qual == NIL); Assert(plan->qual == NIL);
break; break;
@ -552,7 +554,6 @@ adjust_plan_varnos(Plan *plan, int rtoffset)
* or quals. It does have live expressions for limit/offset, * or quals. It does have live expressions for limit/offset,
* however. * however.
*/ */
plan->targetlist = copyObject(plan->targetlist);
adjust_expr_varnos((Node *) plan->targetlist, rtoffset); adjust_expr_varnos((Node *) plan->targetlist, rtoffset);
Assert(plan->qual == NIL); Assert(plan->qual == NIL);
adjust_expr_varnos(((Limit *) plan)->limitOffset, rtoffset); 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); adjust_expr_varnos(((Result *) plan)->resconstantqual, rtoffset);
break; break;
case T_Append: 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); adjust_expr_varnos((Node *) plan->targetlist, rtoffset);
Assert(plan->qual == NIL); Assert(plan->qual == NIL);
foreach(l, ((Append *) plan)->appendplans) 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, * The inner side is a bitmap scan plan. Fix the top node,
* and recurse to get the lower nodes. * 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; BitmapHeapScan *innerscan = (BitmapHeapScan *) inner_plan;
Index innerrel = innerscan->scan.scanrelid;
List *bitmapqualorig = innerscan->bitmapqualorig; 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) 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, innerscan->bitmapqualorig = join_references(bitmapqualorig,
rtable, rtable,
outer_itlist, outer_itlist,
NULL, NULL,
innerrel); innerrel);
/* /*
* We must fix the inner qpqual too, if it has join * We must fix the inner qpqual too, if it has join
* clauses (this could happen if special operators are * clauses (this could happen if special operators are
* involved: some indexquals may get rechecked as qpquals). * involved: some indexquals may get rechecked as qpquals).
*/ */
if (NumRelids((Node *) inner_plan->qual) > 1) if (NumRelids((Node *) inner_plan->qual) > 1)
inner_plan->qual = join_references(inner_plan->qual, inner_plan->qual = join_references(inner_plan->qual,
rtable, rtable,
outer_itlist, outer_itlist,
NULL, NULL,
innerrel); innerrel);
/* Now recurse */ /* Now recurse */
set_inner_join_references(inner_plan->lefttree, set_inner_join_references(inner_plan->lefttree,
rtable, rtable,
outer_itlist); outer_itlist);
}
} }
else if (IsA(inner_plan, BitmapAnd)) else if (IsA(inner_plan, BitmapAnd))
{ {