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

Commit904b171

Browse files
committed
Fix MULTIEXPR_SUBLINK with partitioned target tables, yet again.
We already tried to fix this in commits3f7323c et al (and follow-onfixes), but now it emerges that there are still unfixed cases;moreover, these cases affect all branches not only pre-v14. I thoughtwe had eliminated all cases of making multiple clones of an UPDATE'starget list when we nuked inheritance_planner. But it turns out westill do that in some partitioned-UPDATE cases, notably includingINSERT ... ON CONFLICT UPDATE, because ExecInitPartitionInfo thinksit's okay to clone and modify the parent's targetlist.This fix is based on a suggestion from Andres Freund: let's stopabusing the ParamExecData.execPlan mechanism, which was only evermeant to handle initplans, and instead solve the execution timingproblem by having the expression compiler move MULTIEXPR_SUBLINK stepsto the front of their expression step lists. This is feasible because(a) all branches still in support compile the entire targetlist ofan UPDATE into a single ExprState, and (b) we know that allMULTIEXPR_SUBLINKs do need to be evaluated --- none could be buriedinside a CASE, for example. There is a minor semantics changeconcerning the order of execution of the MULTIEXPR's subquery versusother parts of the parent targetlist, but that seems like somethingwe can get away with. By doing that, we no longer need to worryabout whether different clones of a MULTIEXPR_SUBLINK share outputParams; their usage of that data structure won't overlap.Per bug #17800 from Alexander Lakhin. Back-patch to all supportedbranches. In v13 and earlier, we can revert3f7323c and follow-onfixes; however, I chose to keep the SubPlan.subLinkId field addedinccbb54c. We don't need that anymore in the core code, but it'scheap enough to fill, and removing a plan node field in a minorrelease seems like it'd be asking for trouble.Andres Freund and Tom LaneDiscussion:https://postgr.es/m/17800-ff90866b3906c964@postgresql.org
1 parent4fd093a commit904b171

File tree

8 files changed

+306
-327
lines changed

8 files changed

+306
-327
lines changed

‎src/backend/executor/execExpr.c‎

Lines changed: 107 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -48,22 +48,25 @@
4848
#include"utils/typcache.h"
4949

5050

51-
typedefstructLastAttnumInfo
51+
typedefstructExprSetupInfo
5252
{
53+
/* Highest attribute numbers fetched from inner/outer/scan tuple slots: */
5354
AttrNumberlast_inner;
5455
AttrNumberlast_outer;
5556
AttrNumberlast_scan;
56-
}LastAttnumInfo;
57+
/* MULTIEXPR SubPlan nodes appearing in the expression: */
58+
List*multiexpr_subplans;
59+
}ExprSetupInfo;
5760

5861
staticvoidExecReadyExpr(ExprState*state);
5962
staticvoidExecInitExprRec(Expr*node,ExprState*state,
6063
Datum*resv,bool*resnull);
6164
staticvoidExecInitFunc(ExprEvalStep*scratch,Expr*node,List*args,
6265
Oidfuncid,Oidinputcollid,
6366
ExprState*state);
64-
staticvoidExecInitExprSlots(ExprState*state,Node*node);
65-
staticvoidExecPushExprSlots(ExprState*state,LastAttnumInfo*info);
66-
staticboolget_last_attnums_walker(Node*node,LastAttnumInfo*info);
67+
staticvoidExecCreateExprSetupSteps(ExprState*state,Node*node);
68+
staticvoidExecPushExprSetupSteps(ExprState*state,ExprSetupInfo*info);
69+
staticboolexpr_setup_walker(Node*node,ExprSetupInfo*info);
6770
staticvoidExecComputeSlotInfo(ExprState*state,ExprEvalStep*op);
6871
staticvoidExecInitWholeRowVar(ExprEvalStep*scratch,Var*variable,
6972
ExprState*state);
@@ -132,8 +135,8 @@ ExecInitExpr(Expr *node, PlanState *parent)
132135
state->parent=parent;
133136
state->ext_params=NULL;
134137

135-
/* InsertEEOP_*_FETCHSOME steps as needed */
136-
ExecInitExprSlots(state, (Node*)node);
138+
/* Insertsetup steps as needed */
139+
ExecCreateExprSetupSteps(state, (Node*)node);
137140

138141
/* Compile the expression proper */
139142
ExecInitExprRec(node,state,&state->resvalue,&state->resnull);
@@ -169,8 +172,8 @@ ExecInitExprWithParams(Expr *node, ParamListInfo ext_params)
169172
state->parent=NULL;
170173
state->ext_params=ext_params;
171174

172-
/* InsertEEOP_*_FETCHSOME steps as needed */
173-
ExecInitExprSlots(state, (Node*)node);
175+
/* Insertsetup steps as needed */
176+
ExecCreateExprSetupSteps(state, (Node*)node);
174177

175178
/* Compile the expression proper */
176179
ExecInitExprRec(node,state,&state->resvalue,&state->resnull);
@@ -224,8 +227,8 @@ ExecInitQual(List *qual, PlanState *parent)
224227
/* mark expression as to be used with ExecQual() */
225228
state->flags=EEO_FLAG_IS_QUAL;
226229

227-
/* InsertEEOP_*_FETCHSOME steps as needed */
228-
ExecInitExprSlots(state, (Node*)qual);
230+
/* Insertsetup steps as needed */
231+
ExecCreateExprSetupSteps(state, (Node*)qual);
229232

230233
/*
231234
* ExecQual() needs to return false for an expression returning NULL. That
@@ -394,8 +397,8 @@ ExecBuildProjectionInfoExt(List *targetList,
394397

395398
state->resultslot=slot;
396399

397-
/* InsertEEOP_*_FETCHSOME steps as needed */
398-
ExecInitExprSlots(state, (Node*)targetList);
400+
/* Insertsetup steps as needed */
401+
ExecCreateExprSetupSteps(state, (Node*)targetList);
399402

400403
/* Now compile each tlist column */
401404
foreach(lc,targetList)
@@ -1117,6 +1120,21 @@ ExecInitExprRec(Expr *node, ExprState *state,
11171120
SubPlan*subplan= (SubPlan*)node;
11181121
SubPlanState*sstate;
11191122

1123+
/*
1124+
* Real execution of a MULTIEXPR SubPlan has already been
1125+
* done. What we have to do here is return a dummy NULL record
1126+
* value in case this targetlist element is assigned
1127+
* someplace.
1128+
*/
1129+
if (subplan->subLinkType==MULTIEXPR_SUBLINK)
1130+
{
1131+
scratch.opcode=EEOP_CONST;
1132+
scratch.d.constval.value= (Datum)0;
1133+
scratch.d.constval.isnull= true;
1134+
ExprEvalPushStep(state,&scratch);
1135+
break;
1136+
}
1137+
11201138
if (!state->parent)
11211139
elog(ERROR,"SubPlan found with no parent plan");
11221140

@@ -2285,36 +2303,38 @@ ExecInitFunc(ExprEvalStep *scratch, Expr *node, List *args, Oid funcid,
22852303
}
22862304

22872305
/*
2288-
* Add expression stepsdeforming the ExprState'sinner/outer/scan slots
2289-
*as much as required by the expression.
2306+
* Add expression stepsperforming setup that'sneeded before any of the
2307+
*main execution of the expression.
22902308
*/
22912309
staticvoid
2292-
ExecInitExprSlots(ExprState*state,Node*node)
2310+
ExecCreateExprSetupSteps(ExprState*state,Node*node)
22932311
{
2294-
LastAttnumInfoinfo= {0,0,0};
2312+
ExprSetupInfoinfo= {0,0,0,NIL};
22952313

2296-
/*
2297-
* Figure out which attributes we're going to need.
2298-
*/
2299-
get_last_attnums_walker(node,&info);
2314+
/* Prescan to find out what we need. */
2315+
expr_setup_walker(node,&info);
23002316

2301-
ExecPushExprSlots(state,&info);
2317+
/* And generate those steps. */
2318+
ExecPushExprSetupSteps(state,&info);
23022319
}
23032320

23042321
/*
2305-
* Add steps deforming the ExprState's inner/out/scan slots as much as
2306-
* indicated by info. This is useful when building an ExprState covering more
2307-
* than one expression.
2322+
* Add steps performing expression setup as indicated by "info".
2323+
* This is useful when building an ExprState covering more than one expression.
23082324
*/
23092325
staticvoid
2310-
ExecPushExprSlots(ExprState*state,LastAttnumInfo*info)
2326+
ExecPushExprSetupSteps(ExprState*state,ExprSetupInfo*info)
23112327
{
23122328
ExprEvalStepscratch= {0};
2329+
ListCell*lc;
23132330

23142331
scratch.resvalue=NULL;
23152332
scratch.resnull=NULL;
23162333

2317-
/* Emit steps as needed */
2334+
/*
2335+
* Add steps deforming the ExprState's inner/outer/scan slots as much as
2336+
* required by any Vars appearing in the expression.
2337+
*/
23182338
if (info->last_inner>0)
23192339
{
23202340
scratch.opcode=EEOP_INNER_FETCHSOME;
@@ -2345,13 +2365,48 @@ ExecPushExprSlots(ExprState *state, LastAttnumInfo *info)
23452365
ExecComputeSlotInfo(state,&scratch);
23462366
ExprEvalPushStep(state,&scratch);
23472367
}
2368+
2369+
/*
2370+
* Add steps to execute any MULTIEXPR SubPlans appearing in the
2371+
* expression. We need to evaluate these before any of the Params
2372+
* referencing their outputs are used, but after we've prepared for any
2373+
* Var references they may contain. (There cannot be cross-references
2374+
* between MULTIEXPR SubPlans, so we needn't worry about their order.)
2375+
*/
2376+
foreach(lc,info->multiexpr_subplans)
2377+
{
2378+
SubPlan*subplan= (SubPlan*)lfirst(lc);
2379+
SubPlanState*sstate;
2380+
2381+
Assert(subplan->subLinkType==MULTIEXPR_SUBLINK);
2382+
2383+
/* This should match what ExecInitExprRec does for other SubPlans: */
2384+
2385+
if (!state->parent)
2386+
elog(ERROR,"SubPlan found with no parent plan");
2387+
2388+
sstate=ExecInitSubPlan(subplan,state->parent);
2389+
2390+
/* add SubPlanState nodes to state->parent->subPlan */
2391+
state->parent->subPlan=lappend(state->parent->subPlan,
2392+
sstate);
2393+
2394+
scratch.opcode=EEOP_SUBPLAN;
2395+
scratch.d.subplan.sstate=sstate;
2396+
2397+
/* The result can be ignored, but we better put it somewhere */
2398+
scratch.resvalue=&state->resvalue;
2399+
scratch.resnull=&state->resnull;
2400+
2401+
ExprEvalPushStep(state,&scratch);
2402+
}
23482403
}
23492404

23502405
/*
2351-
*get_last_attnums_walker: expression walker forExecInitExprSlots
2406+
*expr_setup_walker: expression walker forExecCreateExprSetupSteps
23522407
*/
23532408
staticbool
2354-
get_last_attnums_walker(Node*node,LastAttnumInfo*info)
2409+
expr_setup_walker(Node*node,ExprSetupInfo*info)
23552410
{
23562411
if (node==NULL)
23572412
return false;
@@ -2379,6 +2434,16 @@ get_last_attnums_walker(Node *node, LastAttnumInfo *info)
23792434
return false;
23802435
}
23812436

2437+
/* Collect all MULTIEXPR SubPlans, too */
2438+
if (IsA(node,SubPlan))
2439+
{
2440+
SubPlan*subplan= (SubPlan*)node;
2441+
2442+
if (subplan->subLinkType==MULTIEXPR_SUBLINK)
2443+
info->multiexpr_subplans=lappend(info->multiexpr_subplans,
2444+
subplan);
2445+
}
2446+
23822447
/*
23832448
* Don't examine the arguments or filters of Aggrefs or WindowFuncs,
23842449
* because those do not represent expressions to be evaluated within the
@@ -2391,7 +2456,7 @@ get_last_attnums_walker(Node *node, LastAttnumInfo *info)
23912456
return false;
23922457
if (IsA(node,GroupingFunc))
23932458
return false;
2394-
returnexpression_tree_walker(node,get_last_attnums_walker,
2459+
returnexpression_tree_walker(node,expr_setup_walker,
23952460
(void*)info);
23962461
}
23972462

@@ -2968,7 +3033,7 @@ ExecBuildAggTrans(AggState *aggstate, AggStatePerPhase phase,
29683033
inttransno=0;
29693034
intsetoff=0;
29703035
boolisCombine=DO_AGGSPLIT_COMBINE(aggstate->aggsplit);
2971-
LastAttnumInfodeform= {0,0,0};
3036+
ExprSetupInfodeform= {0,0,0,NIL};
29723037

29733038
state->expr= (Expr*)aggstate;
29743039
state->parent=parent;
@@ -2984,18 +3049,18 @@ ExecBuildAggTrans(AggState *aggstate, AggStatePerPhase phase,
29843049
{
29853050
AggStatePerTranspertrans=&aggstate->pertrans[transno];
29863051

2987-
get_last_attnums_walker((Node*)pertrans->aggref->aggdirectargs,
2988-
&deform);
2989-
get_last_attnums_walker((Node*)pertrans->aggref->args,
2990-
&deform);
2991-
get_last_attnums_walker((Node*)pertrans->aggref->aggorder,
2992-
&deform);
2993-
get_last_attnums_walker((Node*)pertrans->aggref->aggdistinct,
2994-
&deform);
2995-
get_last_attnums_walker((Node*)pertrans->aggref->aggfilter,
2996-
&deform);
3052+
expr_setup_walker((Node*)pertrans->aggref->aggdirectargs,
3053+
&deform);
3054+
expr_setup_walker((Node*)pertrans->aggref->args,
3055+
&deform);
3056+
expr_setup_walker((Node*)pertrans->aggref->aggorder,
3057+
&deform);
3058+
expr_setup_walker((Node*)pertrans->aggref->aggdistinct,
3059+
&deform);
3060+
expr_setup_walker((Node*)pertrans->aggref->aggfilter,
3061+
&deform);
29973062
}
2998-
ExecPushExprSlots(state,&deform);
3063+
ExecPushExprSetupSteps(state,&deform);
29993064

30003065
/*
30013066
* Emit instructions for each transition value / grouping set combination.

0 commit comments

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