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

Commit1e199c2

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 parent39ad791 commit1e199c2

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
@@ -50,22 +50,25 @@
5050
#include"utils/typcache.h"
5151

5252

53-
typedefstructLastAttnumInfo
53+
typedefstructExprSetupInfo
5454
{
55+
/* Highest attribute numbers fetched from inner/outer/scan tuple slots: */
5556
AttrNumberlast_inner;
5657
AttrNumberlast_outer;
5758
AttrNumberlast_scan;
58-
}LastAttnumInfo;
59+
/* MULTIEXPR SubPlan nodes appearing in the expression: */
60+
List*multiexpr_subplans;
61+
}ExprSetupInfo;
5962

6063
staticvoidExecReadyExpr(ExprState*state);
6164
staticvoidExecInitExprRec(Expr*node,ExprState*state,
6265
Datum*resv,bool*resnull);
6366
staticvoidExecInitFunc(ExprEvalStep*scratch,Expr*node,List*args,
6467
Oidfuncid,Oidinputcollid,
6568
ExprState*state);
66-
staticvoidExecInitExprSlots(ExprState*state,Node*node);
67-
staticvoidExecPushExprSlots(ExprState*state,LastAttnumInfo*info);
68-
staticboolget_last_attnums_walker(Node*node,LastAttnumInfo*info);
69+
staticvoidExecCreateExprSetupSteps(ExprState*state,Node*node);
70+
staticvoidExecPushExprSetupSteps(ExprState*state,ExprSetupInfo*info);
71+
staticboolexpr_setup_walker(Node*node,ExprSetupInfo*info);
6972
staticboolExecComputeSlotInfo(ExprState*state,ExprEvalStep*op);
7073
staticvoidExecInitWholeRowVar(ExprEvalStep*scratch,Var*variable,
7174
ExprState*state);
@@ -135,8 +138,8 @@ ExecInitExpr(Expr *node, PlanState *parent)
135138
state->parent=parent;
136139
state->ext_params=NULL;
137140

138-
/* InsertEEOP_*_FETCHSOME steps as needed */
139-
ExecInitExprSlots(state, (Node*)node);
141+
/* Insertsetup steps as needed */
142+
ExecCreateExprSetupSteps(state, (Node*)node);
140143

141144
/* Compile the expression proper */
142145
ExecInitExprRec(node,state,&state->resvalue,&state->resnull);
@@ -172,8 +175,8 @@ ExecInitExprWithParams(Expr *node, ParamListInfo ext_params)
172175
state->parent=NULL;
173176
state->ext_params=ext_params;
174177

175-
/* InsertEEOP_*_FETCHSOME steps as needed */
176-
ExecInitExprSlots(state, (Node*)node);
178+
/* Insertsetup steps as needed */
179+
ExecCreateExprSetupSteps(state, (Node*)node);
177180

178181
/* Compile the expression proper */
179182
ExecInitExprRec(node,state,&state->resvalue,&state->resnull);
@@ -227,8 +230,8 @@ ExecInitQual(List *qual, PlanState *parent)
227230
/* mark expression as to be used with ExecQual() */
228231
state->flags=EEO_FLAG_IS_QUAL;
229232

230-
/* InsertEEOP_*_FETCHSOME steps as needed */
231-
ExecInitExprSlots(state, (Node*)qual);
233+
/* Insertsetup steps as needed */
234+
ExecCreateExprSetupSteps(state, (Node*)qual);
232235

233236
/*
234237
* ExecQual() needs to return false for an expression returning NULL. That
@@ -397,8 +400,8 @@ ExecBuildProjectionInfoExt(List *targetList,
397400

398401
state->resultslot=slot;
399402

400-
/* InsertEEOP_*_FETCHSOME steps as needed */
401-
ExecInitExprSlots(state, (Node*)targetList);
403+
/* Insertsetup steps as needed */
404+
ExecCreateExprSetupSteps(state, (Node*)targetList);
402405

403406
/* Now compile each tlist column */
404407
foreach(lc,targetList)
@@ -1119,6 +1122,21 @@ ExecInitExprRec(Expr *node, ExprState *state,
11191122
SubPlan*subplan= (SubPlan*)node;
11201123
SubPlanState*sstate;
11211124

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

@@ -2287,36 +2305,38 @@ ExecInitFunc(ExprEvalStep *scratch, Expr *node, List *args, Oid funcid,
22872305
}
22882306

22892307
/*
2290-
* Add expression stepsdeforming the ExprState'sinner/outer/scan slots
2291-
*as much as required by the expression.
2308+
* Add expression stepsperforming setup that'sneeded before any of the
2309+
*main execution of the expression.
22922310
*/
22932311
staticvoid
2294-
ExecInitExprSlots(ExprState*state,Node*node)
2312+
ExecCreateExprSetupSteps(ExprState*state,Node*node)
22952313
{
2296-
LastAttnumInfoinfo= {0,0,0};
2314+
ExprSetupInfoinfo= {0,0,0,NIL};
22972315

2298-
/*
2299-
* Figure out which attributes we're going to need.
2300-
*/
2301-
get_last_attnums_walker(node,&info);
2316+
/* Prescan to find out what we need. */
2317+
expr_setup_walker(node,&info);
23022318

2303-
ExecPushExprSlots(state,&info);
2319+
/* And generate those steps. */
2320+
ExecPushExprSetupSteps(state,&info);
23042321
}
23052322

23062323
/*
2307-
* Add steps deforming the ExprState's inner/out/scan slots as much as
2308-
* indicated by info. This is useful when building an ExprState covering more
2309-
* than one expression.
2324+
* Add steps performing expression setup as indicated by "info".
2325+
* This is useful when building an ExprState covering more than one expression.
23102326
*/
23112327
staticvoid
2312-
ExecPushExprSlots(ExprState*state,LastAttnumInfo*info)
2328+
ExecPushExprSetupSteps(ExprState*state,ExprSetupInfo*info)
23132329
{
23142330
ExprEvalStepscratch= {0};
2331+
ListCell*lc;
23152332

23162333
scratch.resvalue=NULL;
23172334
scratch.resnull=NULL;
23182335

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

23522407
/*
2353-
*get_last_attnums_walker: expression walker forExecInitExprSlots
2408+
*expr_setup_walker: expression walker forExecCreateExprSetupSteps
23542409
*/
23552410
staticbool
2356-
get_last_attnums_walker(Node*node,LastAttnumInfo*info)
2411+
expr_setup_walker(Node*node,ExprSetupInfo*info)
23572412
{
23582413
if (node==NULL)
23592414
return false;
@@ -2381,6 +2436,16 @@ get_last_attnums_walker(Node *node, LastAttnumInfo *info)
23812436
return false;
23822437
}
23832438

2439+
/* Collect all MULTIEXPR SubPlans, too */
2440+
if (IsA(node,SubPlan))
2441+
{
2442+
SubPlan*subplan= (SubPlan*)node;
2443+
2444+
if (subplan->subLinkType==MULTIEXPR_SUBLINK)
2445+
info->multiexpr_subplans=lappend(info->multiexpr_subplans,
2446+
subplan);
2447+
}
2448+
23842449
/*
23852450
* Don't examine the arguments or filters of Aggrefs or WindowFuncs,
23862451
* because those do not represent expressions to be evaluated within the
@@ -2393,7 +2458,7 @@ get_last_attnums_walker(Node *node, LastAttnumInfo *info)
23932458
return false;
23942459
if (IsA(node,GroupingFunc))
23952460
return false;
2396-
returnexpression_tree_walker(node,get_last_attnums_walker,
2461+
returnexpression_tree_walker(node,expr_setup_walker,
23972462
(void*)info);
23982463
}
23992464

@@ -2985,7 +3050,7 @@ ExecBuildAggTrans(AggState *aggstate, AggStatePerPhase phase,
29853050
PlanState*parent=&aggstate->ss.ps;
29863051
ExprEvalStepscratch= {0};
29873052
boolisCombine=DO_AGGSPLIT_COMBINE(aggstate->aggsplit);
2988-
LastAttnumInfodeform= {0,0,0};
3053+
ExprSetupInfodeform= {0,0,0,NIL};
29893054

29903055
state->expr= (Expr*)aggstate;
29913056
state->parent=parent;
@@ -3001,18 +3066,18 @@ ExecBuildAggTrans(AggState *aggstate, AggStatePerPhase phase,
30013066
{
30023067
AggStatePerTranspertrans=&aggstate->pertrans[transno];
30033068

3004-
get_last_attnums_walker((Node*)pertrans->aggref->aggdirectargs,
3005-
&deform);
3006-
get_last_attnums_walker((Node*)pertrans->aggref->args,
3007-
&deform);
3008-
get_last_attnums_walker((Node*)pertrans->aggref->aggorder,
3009-
&deform);
3010-
get_last_attnums_walker((Node*)pertrans->aggref->aggdistinct,
3011-
&deform);
3012-
get_last_attnums_walker((Node*)pertrans->aggref->aggfilter,
3013-
&deform);
3069+
expr_setup_walker((Node*)pertrans->aggref->aggdirectargs,
3070+
&deform);
3071+
expr_setup_walker((Node*)pertrans->aggref->args,
3072+
&deform);
3073+
expr_setup_walker((Node*)pertrans->aggref->aggorder,
3074+
&deform);
3075+
expr_setup_walker((Node*)pertrans->aggref->aggdistinct,
3076+
&deform);
3077+
expr_setup_walker((Node*)pertrans->aggref->aggfilter,
3078+
&deform);
30143079
}
3015-
ExecPushExprSlots(state,&deform);
3080+
ExecPushExprSetupSteps(state,&deform);
30163081

30173082
/*
30183083
* Emit instructions for each transition value / grouping set combination.

0 commit comments

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