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

Commita033f91

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 parent8e5b4e0 commita033f91

File tree

5 files changed

+332
-119
lines changed

5 files changed

+332
-119
lines changed

‎src/backend/executor/execExpr.c

Lines changed: 112 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -51,22 +51,25 @@
5151
#include"utils/typcache.h"
5252

5353

54-
typedefstructLastAttnumInfo
54+
typedefstructExprSetupInfo
5555
{
56+
/* Highest attribute numbers fetched from inner/outer/scan tuple slots: */
5657
AttrNumberlast_inner;
5758
AttrNumberlast_outer;
5859
AttrNumberlast_scan;
59-
}LastAttnumInfo;
60+
/* MULTIEXPR SubPlan nodes appearing in the expression: */
61+
List*multiexpr_subplans;
62+
}ExprSetupInfo;
6063

6164
staticvoidExecReadyExpr(ExprState*state);
6265
staticvoidExecInitExprRec(Expr*node,ExprState*state,
6366
Datum*resv,bool*resnull);
6467
staticvoidExecInitFunc(ExprEvalStep*scratch,Expr*node,List*args,
6568
Oidfuncid,Oidinputcollid,
6669
ExprState*state);
67-
staticvoidExecInitExprSlots(ExprState*state,Node*node);
68-
staticvoidExecPushExprSlots(ExprState*state,LastAttnumInfo*info);
69-
staticboolget_last_attnums_walker(Node*node,LastAttnumInfo*info);
70+
staticvoidExecCreateExprSetupSteps(ExprState*state,Node*node);
71+
staticvoidExecPushExprSetupSteps(ExprState*state,ExprSetupInfo*info);
72+
staticboolexpr_setup_walker(Node*node,ExprSetupInfo*info);
7073
staticboolExecComputeSlotInfo(ExprState*state,ExprEvalStep*op);
7174
staticvoidExecInitWholeRowVar(ExprEvalStep*scratch,Var*variable,
7275
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
@@ -371,8 +374,8 @@ ExecBuildProjectionInfo(List *targetList,
371374

372375
state->resultslot=slot;
373376

374-
/* InsertEEOP_*_FETCHSOME steps as needed */
375-
ExecInitExprSlots(state, (Node*)targetList);
377+
/* Insertsetup steps as needed */
378+
ExecCreateExprSetupSteps(state, (Node*)targetList);
376379

377380
/* Now compile each tlist column */
378381
foreach(lc,targetList)
@@ -523,7 +526,7 @@ ExecBuildUpdateProjection(List *targetList,
523526
intnAssignableCols;
524527
boolsawJunk;
525528
Bitmapset*assignedCols;
526-
LastAttnumInfodeform= {0,0,0};
529+
ExprSetupInfodeform= {0,0,0,NIL};
527530
ExprEvalStepscratch= {0};
528531
intouterattnum;
529532
ListCell*lc,
@@ -602,17 +605,18 @@ ExecBuildUpdateProjection(List *targetList,
602605
* number of columns of the "outer" tuple.
603606
*/
604607
if (evalTargetList)
605-
get_last_attnums_walker((Node*)targetList,&deform);
608+
expr_setup_walker((Node*)targetList,&deform);
606609
else
607610
deform.last_outer=nAssignableCols;
608611

609-
ExecPushExprSlots(state,&deform);
612+
ExecPushExprSetupSteps(state,&deform);
610613

611614
/*
612615
* Now generate code to evaluate the tlist's assignable expressions or
613616
* fetch them from the outer tuple, incidentally validating that they'll
614617
* be of the right data type. The checks above ensure that the forboth()
615-
* will iterate over exactly the non-junk columns.
618+
* will iterate over exactly the non-junk columns. Note that we don't
619+
* bother evaluating any remaining resjunk columns.
616620
*/
617621
outerattnum=0;
618622
forboth(lc,targetList,lc2,targetColnos)
@@ -675,22 +679,6 @@ ExecBuildUpdateProjection(List *targetList,
675679
outerattnum++;
676680
}
677681

678-
/*
679-
* If we're evaluating the tlist, must evaluate any resjunk columns too.
680-
* (This matters for things like MULTIEXPR_SUBLINK SubPlans.)
681-
*/
682-
if (evalTargetList)
683-
{
684-
for_each_cell(lc,targetList,lc)
685-
{
686-
TargetEntry*tle=lfirst_node(TargetEntry,lc);
687-
688-
Assert(tle->resjunk);
689-
ExecInitExprRec(tle->expr,state,
690-
&state->resvalue,&state->resnull);
691-
}
692-
}
693-
694682
/*
695683
* Now generate code to copy over any old columns that were not assigned
696684
* to, and to ensure that dropped columns are set to NULL.
@@ -1401,6 +1389,21 @@ ExecInitExprRec(Expr *node, ExprState *state,
14011389
SubPlan*subplan= (SubPlan*)node;
14021390
SubPlanState*sstate;
14031391

1392+
/*
1393+
* Real execution of a MULTIEXPR SubPlan has already been
1394+
* done. What we have to do here is return a dummy NULL record
1395+
* value in case this targetlist element is assigned
1396+
* someplace.
1397+
*/
1398+
if (subplan->subLinkType==MULTIEXPR_SUBLINK)
1399+
{
1400+
scratch.opcode=EEOP_CONST;
1401+
scratch.d.constval.value= (Datum)0;
1402+
scratch.d.constval.isnull= true;
1403+
ExprEvalPushStep(state,&scratch);
1404+
break;
1405+
}
1406+
14041407
if (!state->parent)
14051408
elog(ERROR,"SubPlan found with no parent plan");
14061409

@@ -2552,36 +2555,38 @@ ExecInitFunc(ExprEvalStep *scratch, Expr *node, List *args, Oid funcid,
25522555
}
25532556

25542557
/*
2555-
* Add expression stepsdeforming the ExprState'sinner/outer/scan slots
2556-
*as much as required by the expression.
2558+
* Add expression stepsperforming setup that'sneeded before any of the
2559+
*main execution of the expression.
25572560
*/
25582561
staticvoid
2559-
ExecInitExprSlots(ExprState*state,Node*node)
2562+
ExecCreateExprSetupSteps(ExprState*state,Node*node)
25602563
{
2561-
LastAttnumInfoinfo= {0,0,0};
2564+
ExprSetupInfoinfo= {0,0,0,NIL};
25622565

2563-
/*
2564-
* Figure out which attributes we're going to need.
2565-
*/
2566-
get_last_attnums_walker(node,&info);
2566+
/* Prescan to find out what we need. */
2567+
expr_setup_walker(node,&info);
25672568

2568-
ExecPushExprSlots(state,&info);
2569+
/* And generate those steps. */
2570+
ExecPushExprSetupSteps(state,&info);
25692571
}
25702572

25712573
/*
2572-
* Add steps deforming the ExprState's inner/out/scan slots as much as
2573-
* indicated by info. This is useful when building an ExprState covering more
2574-
* than one expression.
2574+
* Add steps performing expression setup as indicated by "info".
2575+
* This is useful when building an ExprState covering more than one expression.
25752576
*/
25762577
staticvoid
2577-
ExecPushExprSlots(ExprState*state,LastAttnumInfo*info)
2578+
ExecPushExprSetupSteps(ExprState*state,ExprSetupInfo*info)
25782579
{
25792580
ExprEvalStepscratch= {0};
2581+
ListCell*lc;
25802582

25812583
scratch.resvalue=NULL;
25822584
scratch.resnull=NULL;
25832585

2584-
/* Emit steps as needed */
2586+
/*
2587+
* Add steps deforming the ExprState's inner/outer/scan slots as much as
2588+
* required by any Vars appearing in the expression.
2589+
*/
25852590
if (info->last_inner>0)
25862591
{
25872592
scratch.opcode=EEOP_INNER_FETCHSOME;
@@ -2612,13 +2617,48 @@ ExecPushExprSlots(ExprState *state, LastAttnumInfo *info)
26122617
if (ExecComputeSlotInfo(state,&scratch))
26132618
ExprEvalPushStep(state,&scratch);
26142619
}
2620+
2621+
/*
2622+
* Add steps to execute any MULTIEXPR SubPlans appearing in the
2623+
* expression. We need to evaluate these before any of the Params
2624+
* referencing their outputs are used, but after we've prepared for any
2625+
* Var references they may contain. (There cannot be cross-references
2626+
* between MULTIEXPR SubPlans, so we needn't worry about their order.)
2627+
*/
2628+
foreach(lc,info->multiexpr_subplans)
2629+
{
2630+
SubPlan*subplan= (SubPlan*)lfirst(lc);
2631+
SubPlanState*sstate;
2632+
2633+
Assert(subplan->subLinkType==MULTIEXPR_SUBLINK);
2634+
2635+
/* This should match what ExecInitExprRec does for other SubPlans: */
2636+
2637+
if (!state->parent)
2638+
elog(ERROR,"SubPlan found with no parent plan");
2639+
2640+
sstate=ExecInitSubPlan(subplan,state->parent);
2641+
2642+
/* add SubPlanState nodes to state->parent->subPlan */
2643+
state->parent->subPlan=lappend(state->parent->subPlan,
2644+
sstate);
2645+
2646+
scratch.opcode=EEOP_SUBPLAN;
2647+
scratch.d.subplan.sstate=sstate;
2648+
2649+
/* The result can be ignored, but we better put it somewhere */
2650+
scratch.resvalue=&state->resvalue;
2651+
scratch.resnull=&state->resnull;
2652+
2653+
ExprEvalPushStep(state,&scratch);
2654+
}
26152655
}
26162656

26172657
/*
2618-
*get_last_attnums_walker: expression walker forExecInitExprSlots
2658+
*expr_setup_walker: expression walker forExecCreateExprSetupSteps
26192659
*/
26202660
staticbool
2621-
get_last_attnums_walker(Node*node,LastAttnumInfo*info)
2661+
expr_setup_walker(Node*node,ExprSetupInfo*info)
26222662
{
26232663
if (node==NULL)
26242664
return false;
@@ -2646,6 +2686,16 @@ get_last_attnums_walker(Node *node, LastAttnumInfo *info)
26462686
return false;
26472687
}
26482688

2689+
/* Collect all MULTIEXPR SubPlans, too */
2690+
if (IsA(node,SubPlan))
2691+
{
2692+
SubPlan*subplan= (SubPlan*)node;
2693+
2694+
if (subplan->subLinkType==MULTIEXPR_SUBLINK)
2695+
info->multiexpr_subplans=lappend(info->multiexpr_subplans,
2696+
subplan);
2697+
}
2698+
26492699
/*
26502700
* Don't examine the arguments or filters of Aggrefs or WindowFuncs,
26512701
* because those do not represent expressions to be evaluated within the
@@ -2658,7 +2708,7 @@ get_last_attnums_walker(Node *node, LastAttnumInfo *info)
26582708
return false;
26592709
if (IsA(node,GroupingFunc))
26602710
return false;
2661-
returnexpression_tree_walker(node,get_last_attnums_walker,
2711+
returnexpression_tree_walker(node,expr_setup_walker,
26622712
(void*)info);
26632713
}
26642714

@@ -3277,7 +3327,7 @@ ExecBuildAggTrans(AggState *aggstate, AggStatePerPhase phase,
32773327
PlanState*parent=&aggstate->ss.ps;
32783328
ExprEvalStepscratch= {0};
32793329
boolisCombine=DO_AGGSPLIT_COMBINE(aggstate->aggsplit);
3280-
LastAttnumInfodeform= {0,0,0};
3330+
ExprSetupInfodeform= {0,0,0,NIL};
32813331

32823332
state->expr= (Expr*)aggstate;
32833333
state->parent=parent;
@@ -3293,18 +3343,18 @@ ExecBuildAggTrans(AggState *aggstate, AggStatePerPhase phase,
32933343
{
32943344
AggStatePerTranspertrans=&aggstate->pertrans[transno];
32953345

3296-
get_last_attnums_walker((Node*)pertrans->aggref->aggdirectargs,
3297-
&deform);
3298-
get_last_attnums_walker((Node*)pertrans->aggref->args,
3299-
&deform);
3300-
get_last_attnums_walker((Node*)pertrans->aggref->aggorder,
3301-
&deform);
3302-
get_last_attnums_walker((Node*)pertrans->aggref->aggdistinct,
3303-
&deform);
3304-
get_last_attnums_walker((Node*)pertrans->aggref->aggfilter,
3305-
&deform);
3346+
expr_setup_walker((Node*)pertrans->aggref->aggdirectargs,
3347+
&deform);
3348+
expr_setup_walker((Node*)pertrans->aggref->args,
3349+
&deform);
3350+
expr_setup_walker((Node*)pertrans->aggref->aggorder,
3351+
&deform);
3352+
expr_setup_walker((Node*)pertrans->aggref->aggdistinct,
3353+
&deform);
3354+
expr_setup_walker((Node*)pertrans->aggref->aggfilter,
3355+
&deform);
33063356
}
3307-
ExecPushExprSlots(state,&deform);
3357+
ExecPushExprSetupSteps(state,&deform);
33083358

33093359
/*
33103360
* Emit instructions for each transition value / grouping set combination.

0 commit comments

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