Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit1c72a8a

Browse files
committed
Fix extremely nasty little bug observed when a sub-SELECT appears in
WHERE in a place where it can be part of a nestloop inner indexqual.As the code stood, it put the same physical sub-Plan node into bothindxqual and indxqualorig of the IndexScan plan node. That confusedlater processing in the optimizer (which expected that tracing thesubPlan list would visit each subplan node exactly once), and wouldprobably have blown up in the executor if the planner hadn't choked first.Fix by making the 'fixed' indexqual be a complete deep copy of theoriginal indexqual, rather than trying to share nodes below the topmostoperator node. This had further ramifications though, because we weremaking the aforesaid list of sub-Plan nodes during SS_process_sublinkswhich is run before construction of the 'fixed' indexqual, meaning thatthe copy of the sub-Plan didn't show up in that list. Fix by rearranginglogic so that the sub-Plan list is built by the final set_plan_referencespass, not in SS_process_sublinks. This may sound like a mess, but it'sactually a good deal cleaner now than it was before, because we are nolonger dependent on the assumption that planning will never make a copyof a sub-Plan node.
1 parent8cdabf0 commit1c72a8a

File tree

7 files changed

+194
-134
lines changed

7 files changed

+194
-134
lines changed

‎src/backend/nodes/copyfuncs.c

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,15 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v 1.110 2000/03/22 22:08:32 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v 1.111 2000/04/04 01:21:48 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
1515

1616
#include"postgres.h"
1717

18+
#include"optimizer/clauses.h"
1819
#include"optimizer/planmain.h"
19-
#include"optimizer/subselect.h"
2020

2121

2222
/*
@@ -88,9 +88,10 @@ CopyPlanFields(Plan *from, Plan *newnode)
8888
newnode->locParam=listCopy(from->locParam);
8989
newnode->chgParam=listCopy(from->chgParam);
9090
Node_Copy(from,newnode,initPlan);
91+
/* subPlan list must point to subplans in the new subtree, not the old */
9192
if (from->subPlan!=NIL)
92-
newnode->subPlan=nconc(SS_pull_subplan((Node*)newnode->targetlist),
93-
SS_pull_subplan((Node*)newnode->qual));
93+
newnode->subPlan=nconc(pull_subplans((Node*)newnode->targetlist),
94+
pull_subplans((Node*)newnode->qual));
9495
else
9596
newnode->subPlan=NIL;
9697
newnode->nParamExec=from->nParamExec;
@@ -142,7 +143,7 @@ _copyResult(Result *from)
142143
*/
143144
if (from->plan.subPlan!=NIL)
144145
newnode->plan.subPlan=nconc(newnode->plan.subPlan,
145-
SS_pull_subplan(newnode->resconstantqual));
146+
pull_subplans(newnode->resconstantqual));
146147

147148
returnnewnode;
148149
}
@@ -252,6 +253,17 @@ _copyIndexScan(IndexScan *from)
252253
Node_Copy(from,newnode,indxqualorig);
253254
newnode->indxorderdir=from->indxorderdir;
254255

256+
/*
257+
* We must add subplans in index quals to the new plan's subPlan list
258+
*/
259+
if (from->scan.plan.subPlan!=NIL)
260+
{
261+
newnode->scan.plan.subPlan=nconc(newnode->scan.plan.subPlan,
262+
pull_subplans((Node*)newnode->indxqual));
263+
newnode->scan.plan.subPlan=nconc(newnode->scan.plan.subPlan,
264+
pull_subplans((Node*)newnode->indxqualorig));
265+
}
266+
255267
returnnewnode;
256268
}
257269

@@ -358,6 +370,13 @@ _copyMergeJoin(MergeJoin *from)
358370
*/
359371
Node_Copy(from,newnode,mergeclauses);
360372

373+
/*
374+
* We must add subplans in mergeclauses to the new plan's subPlan list
375+
*/
376+
if (from->join.subPlan!=NIL)
377+
newnode->join.subPlan=nconc(newnode->join.subPlan,
378+
pull_subplans((Node*)newnode->mergeclauses));
379+
361380
returnnewnode;
362381
}
363382

@@ -382,9 +401,15 @@ _copyHashJoin(HashJoin *from)
382401
* ----------------
383402
*/
384403
Node_Copy(from,newnode,hashclauses);
385-
386404
newnode->hashjoinop=from->hashjoinop;
387405

406+
/*
407+
* We must add subplans in hashclauses to the new plan's subPlan list
408+
*/
409+
if (from->join.subPlan!=NIL)
410+
newnode->join.subPlan=nconc(newnode->join.subPlan,
411+
pull_subplans((Node*)newnode->hashclauses));
412+
388413
returnnewnode;
389414
}
390415

‎src/backend/optimizer/plan/createplan.c

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
*
1111
*
1212
* IDENTIFICATION
13-
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v 1.87 2000/03/22 22:08:34 tgl Exp $
13+
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v 1.88 2000/04/04 01:21:47 tgl Exp $
1414
*
1515
*-------------------------------------------------------------------------
1616
*/
@@ -724,7 +724,9 @@ create_hashjoin_node(HashPath *best_path,
724724
* track of which index applies to each subgroup of index qual clauses...
725725
*
726726
* Returns a modified copy of the indexqual list --- the original is not
727-
* changed.
727+
* changed. Note also that the copy shares no substructure with the
728+
* original; this is needed in case there is a subplan in it (we need
729+
* two separate copies of the subplan tree, or things will go awry).
728730
*/
729731

730732
staticList*
@@ -808,11 +810,13 @@ fix_indxqual_sublist(List *indexqual, int baserelid, Oid relam,
808810
get_relattval((Node*)clause,baserelid,
809811
&relid,&attno,&constval,&flag);
810812

811-
/* Copy enough structure to allow commuting and replacing an operand
812-
* without changing original clause.
813+
/* Make a copy that will become the fixed clause.
814+
*
815+
* We used to try to do a shallow copy here, but that fails if there
816+
* is a subplan in the arguments of the opclause. So just do a
817+
* full copy.
813818
*/
814-
newclause=make_clause(clause->opType,clause->oper,
815-
listCopy(clause->args));
819+
newclause= (Expr*)copyObject((Node*)clause);
816820

817821
/* If the indexkey is on the right, commute the clause. */
818822
if ((flag&SEL_RIGHT)==0)
@@ -834,11 +838,7 @@ fix_indxqual_sublist(List *indexqual, int baserelid, Oid relam,
834838
newopno=indexable_operator(newclause,opclass,relam, true);
835839
if (newopno==InvalidOid)
836840
elog(ERROR,"fix_indxqual_sublist: failed to find substitute op");
837-
if (newopno!= ((Oper*)newclause->oper)->opno)
838-
{
839-
newclause->oper= (Node*)copyObject(newclause->oper);
840-
((Oper*)newclause->oper)->opno=newopno;
841-
}
841+
((Oper*)newclause->oper)->opno=newopno;
842842

843843
fixed_qual=lappend(fixed_qual,newclause);
844844
}

‎src/backend/optimizer/plan/setrefs.c

Lines changed: 32 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
*
1010
*
1111
* IDENTIFICATION
12-
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/setrefs.c,v 1.60 2000/01/26 05:56:38 momjian Exp $
12+
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/setrefs.c,v 1.61 2000/04/04 01:21:47 tgl Exp $
1313
*
1414
*-------------------------------------------------------------------------
1515
*/
@@ -59,6 +59,8 @@ static bool fix_opids_walker(Node *node, void *context);
5959
* for the convenience of the executor. We update Vars in upper plan nodes
6060
* to refer to the outputs of their subplans, and we compute regproc OIDs
6161
* for operators (ie, we look up the function that implements each op).
62+
* We must also build lists of all the subplan nodes present in each
63+
* plan node's expression trees.
6264
*
6365
* set_plan_references recursively traverses the whole plan tree.
6466
*
@@ -72,6 +74,11 @@ set_plan_references(Plan *plan)
7274
if (plan==NULL)
7375
return;
7476

77+
/* We must rebuild the plan's list of subplan nodes, since we are
78+
* copying/mutating its expression trees.
79+
*/
80+
plan->subPlan=NIL;
81+
7582
/*
7683
* Plan-type-specific fixes
7784
*/
@@ -83,17 +90,29 @@ set_plan_references(Plan *plan)
8390
caseT_IndexScan:
8491
fix_opids((Node*) ((IndexScan*)plan)->indxqual);
8592
fix_opids((Node*) ((IndexScan*)plan)->indxqualorig);
93+
plan->subPlan=
94+
nconc(plan->subPlan,
95+
pull_subplans((Node*) ((IndexScan*)plan)->indxqual));
96+
plan->subPlan=
97+
nconc(plan->subPlan,
98+
pull_subplans((Node*) ((IndexScan*)plan)->indxqualorig));
8699
break;
87100
caseT_NestLoop:
88101
set_join_references((Join*)plan);
89102
break;
90103
caseT_MergeJoin:
91104
set_join_references((Join*)plan);
92105
fix_opids((Node*) ((MergeJoin*)plan)->mergeclauses);
106+
plan->subPlan=
107+
nconc(plan->subPlan,
108+
pull_subplans((Node*) ((MergeJoin*)plan)->mergeclauses));
93109
break;
94110
caseT_HashJoin:
95111
set_join_references((Join*)plan);
96112
fix_opids((Node*) ((HashJoin*)plan)->hashclauses);
113+
plan->subPlan=
114+
nconc(plan->subPlan,
115+
pull_subplans((Node*) ((HashJoin*)plan)->hashclauses));
97116
break;
98117
caseT_Material:
99118
caseT_Sort:
@@ -119,6 +138,9 @@ set_plan_references(Plan *plan)
119138
if (plan->lefttree!=NULL)
120139
set_uppernode_references(plan, (Index)OUTER);
121140
fix_opids(((Result*)plan)->resconstantqual);
141+
plan->subPlan=
142+
nconc(plan->subPlan,
143+
pull_subplans(((Result*)plan)->resconstantqual));
122144
break;
123145
caseT_Append:
124146
foreach(pl, ((Append*)plan)->appendplans)
@@ -136,10 +158,17 @@ set_plan_references(Plan *plan)
136158
}
137159

138160
/*
139-
* For all plan types, fix operators in targetlist and qual expressions
161+
* For all plan types, fix operators in targetlist and qual expressions,
162+
* and find subplans therein.
140163
*/
141164
fix_opids((Node*)plan->targetlist);
142165
fix_opids((Node*)plan->qual);
166+
plan->subPlan=
167+
nconc(plan->subPlan,
168+
pull_subplans((Node*)plan->targetlist));
169+
plan->subPlan=
170+
nconc(plan->subPlan,
171+
pull_subplans((Node*)plan->qual));
143172

144173
/*
145174
* Now recurse into subplans, if any
@@ -302,31 +331,7 @@ join_references_mutator(Node *node,
302331
*/
303332
if (var->varno!=context->acceptable_rel)
304333
elog(ERROR,"join_references: variable not in subplan target lists");
305-
return (Node*)newvar;/* copy is probably not necessary here... */
306-
}
307-
/*
308-
* expression_tree_mutator will copy SubPlan nodes if given a chance.
309-
* We do not want to do that here, because subselect.c has already
310-
* constructed the initPlan and subPlan lists of the current plan node
311-
* and we mustn't leave those dangling (ie, pointing to different
312-
* copies of the nodes than what's in the targetlist & quals...)
313-
* Instead, alter the SubPlan in-place. Grotty --- is there a better way?
314-
*/
315-
if (is_subplan(node))
316-
{
317-
Expr*expr= (Expr*)node;
318-
SubLink*sublink= ((SubPlan*)expr->oper)->sublink;
319-
320-
/* transform args list (params to be passed to subplan) */
321-
expr->args= (List*)
322-
join_references_mutator((Node*)expr->args,
323-
context);
324-
/* transform sublink's oper list as well */
325-
sublink->oper= (List*)
326-
join_references_mutator((Node*)sublink->oper,
327-
context);
328-
329-
return (Node*)expr;
334+
return (Node*)newvar;
330335
}
331336
returnexpression_tree_mutator(node,
332337
join_references_mutator,
@@ -383,30 +388,6 @@ replace_vars_with_subplan_refs_mutator(Node *node,
383388
newvar->varattno=resdom->resno;
384389
return (Node*)newvar;
385390
}
386-
/*
387-
* expression_tree_mutator will copy SubPlan nodes if given a chance.
388-
* We do not want to do that here, because subselect.c has already
389-
* constructed the initPlan and subPlan lists of the current plan node
390-
* and we mustn't leave those dangling (ie, pointing to different
391-
* copies of the nodes than what's in the targetlist & quals...)
392-
* Instead, alter the SubPlan in-place. Grotty --- is there a better way?
393-
*/
394-
if (is_subplan(node))
395-
{
396-
Expr*expr= (Expr*)node;
397-
SubLink*sublink= ((SubPlan*)expr->oper)->sublink;
398-
399-
/* transform args list (params to be passed to subplan) */
400-
expr->args= (List*)
401-
replace_vars_with_subplan_refs_mutator((Node*)expr->args,
402-
context);
403-
/* transform sublink's oper list as well */
404-
sublink->oper= (List*)
405-
replace_vars_with_subplan_refs_mutator((Node*)sublink->oper,
406-
context);
407-
408-
return (Node*)expr;
409-
}
410391
returnexpression_tree_mutator(node,
411392
replace_vars_with_subplan_refs_mutator,
412393
(void*)context);

‎src/backend/optimizer/plan/subselect.c

Lines changed: 11 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/subselect.c,v 1.33 2000/03/21 05:12:02 tgl Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/optimizer/plan/subselect.c,v 1.34 2000/04/04 01:21:47 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -442,6 +442,12 @@ set_unioni(List *l1, List *l2)
442442
* finalize_primnode: build lists of subplans and params appearing
443443
* in the given expression tree. NOTE: items are added to lists passed in,
444444
* so caller must initialize lists to NIL before first call!
445+
*
446+
* Note: the subplan list that is constructed here and assigned to the
447+
* plan's subPlan field will be replaced with an up-to-date list in
448+
* set_plan_references(). We could almost dispense with building this
449+
* subplan list at all; I believe the only place that uses it is the
450+
* check in make_subplan to see whether a subselect has any subselects.
445451
*/
446452

447453
typedefstructfinalize_primnode_results {
@@ -604,6 +610,10 @@ SS_finalize_plan(Plan *plan)
604610
caseT_IndexScan:
605611
finalize_primnode((Node*) ((IndexScan*)plan)->indxqual,
606612
&results);
613+
/* we need not look at indxqualorig, since it will have the
614+
* same param references as indxqual, and we aren't really
615+
* concerned yet about having a complete subplan list.
616+
*/
607617
break;
608618

609619
caseT_MergeJoin:
@@ -670,32 +680,3 @@ SS_finalize_plan(Plan *plan)
670680

671681
returnresults.paramids;
672682
}
673-
674-
/*
675-
* Construct a list of all subplans found within the given node tree.
676-
*/
677-
678-
staticboolSS_pull_subplan_walker(Node*node,List**listptr);
679-
680-
List*
681-
SS_pull_subplan(Node*expr)
682-
{
683-
List*result=NIL;
684-
685-
SS_pull_subplan_walker(expr,&result);
686-
returnresult;
687-
}
688-
689-
staticbool
690-
SS_pull_subplan_walker(Node*node,List**listptr)
691-
{
692-
if (node==NULL)
693-
return false;
694-
if (is_subplan(node))
695-
{
696-
*listptr=lappend(*listptr, ((Expr*)node)->oper);
697-
/* fall through to check args to subplan */
698-
}
699-
returnexpression_tree_walker(node,SS_pull_subplan_walker,
700-
(void*)listptr);
701-
}

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp