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

Commit87f3667

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 parenta7d71c4 commit87f3667

File tree

5 files changed

+332
-134
lines changed

5 files changed

+332
-134
lines changed

‎src/backend/executor/execExpr.c

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

5454

55-
typedefstructLastAttnumInfo
55+
typedefstructExprSetupInfo
5656
{
57+
/* Highest attribute numbers fetched from inner/outer/scan tuple slots: */
5758
AttrNumberlast_inner;
5859
AttrNumberlast_outer;
5960
AttrNumberlast_scan;
60-
}LastAttnumInfo;
61+
/* MULTIEXPR SubPlan nodes appearing in the expression: */
62+
List*multiexpr_subplans;
63+
}ExprSetupInfo;
6164

6265
staticvoidExecReadyExpr(ExprState*state);
6366
staticvoidExecInitExprRec(Expr*node,ExprState*state,
6467
Datum*resv,bool*resnull);
6568
staticvoidExecInitFunc(ExprEvalStep*scratch,Expr*node,List*args,
6669
Oidfuncid,Oidinputcollid,
6770
ExprState*state);
68-
staticvoidExecInitExprSlots(ExprState*state,Node*node);
69-
staticvoidExecPushExprSlots(ExprState*state,LastAttnumInfo*info);
70-
staticboolget_last_attnums_walker(Node*node,LastAttnumInfo*info);
71+
staticvoidExecCreateExprSetupSteps(ExprState*state,Node*node);
72+
staticvoidExecPushExprSetupSteps(ExprState*state,ExprSetupInfo*info);
73+
staticboolexpr_setup_walker(Node*node,ExprSetupInfo*info);
7174
staticboolExecComputeSlotInfo(ExprState*state,ExprEvalStep*op);
7275
staticvoidExecInitWholeRowVar(ExprEvalStep*scratch,Var*variable,
7376
ExprState*state);
@@ -136,8 +139,8 @@ ExecInitExpr(Expr *node, PlanState *parent)
136139
state->parent=parent;
137140
state->ext_params=NULL;
138141

139-
/* InsertEEOP_*_FETCHSOME steps as needed */
140-
ExecInitExprSlots(state, (Node*)node);
142+
/* Insertsetup steps as needed */
143+
ExecCreateExprSetupSteps(state, (Node*)node);
141144

142145
/* Compile the expression proper */
143146
ExecInitExprRec(node,state,&state->resvalue,&state->resnull);
@@ -173,8 +176,8 @@ ExecInitExprWithParams(Expr *node, ParamListInfo ext_params)
173176
state->parent=NULL;
174177
state->ext_params=ext_params;
175178

176-
/* InsertEEOP_*_FETCHSOME steps as needed */
177-
ExecInitExprSlots(state, (Node*)node);
179+
/* Insertsetup steps as needed */
180+
ExecCreateExprSetupSteps(state, (Node*)node);
178181

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

231-
/* InsertEEOP_*_FETCHSOME steps as needed */
232-
ExecInitExprSlots(state, (Node*)qual);
234+
/* Insertsetup steps as needed */
235+
ExecCreateExprSetupSteps(state, (Node*)qual);
233236

234237
/*
235238
* ExecQual() needs to return false for an expression returning NULL. That
@@ -372,8 +375,8 @@ ExecBuildProjectionInfo(List *targetList,
372375

373376
state->resultslot=slot;
374377

375-
/* InsertEEOP_*_FETCHSOME steps as needed */
376-
ExecInitExprSlots(state, (Node*)targetList);
378+
/* Insertsetup steps as needed */
379+
ExecCreateExprSetupSteps(state, (Node*)targetList);
377380

378381
/* Now compile each tlist column */
379382
foreach(lc,targetList)
@@ -524,7 +527,7 @@ ExecBuildUpdateProjection(List *targetList,
524527
intnAssignableCols;
525528
boolsawJunk;
526529
Bitmapset*assignedCols;
527-
LastAttnumInfodeform= {0,0,0};
530+
ExprSetupInfodeform= {0,0,0,NIL};
528531
ExprEvalStepscratch= {0};
529532
intouterattnum;
530533
ListCell*lc,
@@ -603,17 +606,18 @@ ExecBuildUpdateProjection(List *targetList,
603606
* number of columns of the "outer" tuple.
604607
*/
605608
if (evalTargetList)
606-
get_last_attnums_walker((Node*)targetList,&deform);
609+
expr_setup_walker((Node*)targetList,&deform);
607610
else
608611
deform.last_outer=nAssignableCols;
609612

610-
ExecPushExprSlots(state,&deform);
613+
ExecPushExprSetupSteps(state,&deform);
611614

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

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

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

@@ -2542,36 +2545,38 @@ ExecInitFunc(ExprEvalStep *scratch, Expr *node, List *args, Oid funcid,
25422545
}
25432546

25442547
/*
2545-
* Add expression stepsdeforming the ExprState'sinner/outer/scan slots
2546-
*as much as required by the expression.
2548+
* Add expression stepsperforming setup that'sneeded before any of the
2549+
*main execution of the expression.
25472550
*/
25482551
staticvoid
2549-
ExecInitExprSlots(ExprState*state,Node*node)
2552+
ExecCreateExprSetupSteps(ExprState*state,Node*node)
25502553
{
2551-
LastAttnumInfoinfo= {0,0,0};
2554+
ExprSetupInfoinfo= {0,0,0,NIL};
25522555

2553-
/*
2554-
* Figure out which attributes we're going to need.
2555-
*/
2556-
get_last_attnums_walker(node,&info);
2556+
/* Prescan to find out what we need. */
2557+
expr_setup_walker(node,&info);
25572558

2558-
ExecPushExprSlots(state,&info);
2559+
/* And generate those steps. */
2560+
ExecPushExprSetupSteps(state,&info);
25592561
}
25602562

25612563
/*
2562-
* Add steps deforming the ExprState's inner/out/scan slots as much as
2563-
* indicated by info. This is useful when building an ExprState covering more
2564-
* than one expression.
2564+
* Add steps performing expression setup as indicated by "info".
2565+
* This is useful when building an ExprState covering more than one expression.
25652566
*/
25662567
staticvoid
2567-
ExecPushExprSlots(ExprState*state,LastAttnumInfo*info)
2568+
ExecPushExprSetupSteps(ExprState*state,ExprSetupInfo*info)
25682569
{
25692570
ExprEvalStepscratch= {0};
2571+
ListCell*lc;
25702572

25712573
scratch.resvalue=NULL;
25722574
scratch.resnull=NULL;
25732575

2574-
/* Emit steps as needed */
2576+
/*
2577+
* Add steps deforming the ExprState's inner/outer/scan slots as much as
2578+
* required by any Vars appearing in the expression.
2579+
*/
25752580
if (info->last_inner>0)
25762581
{
25772582
scratch.opcode=EEOP_INNER_FETCHSOME;
@@ -2602,13 +2607,48 @@ ExecPushExprSlots(ExprState *state, LastAttnumInfo *info)
26022607
if (ExecComputeSlotInfo(state,&scratch))
26032608
ExprEvalPushStep(state,&scratch);
26042609
}
2610+
2611+
/*
2612+
* Add steps to execute any MULTIEXPR SubPlans appearing in the
2613+
* expression. We need to evaluate these before any of the Params
2614+
* referencing their outputs are used, but after we've prepared for any
2615+
* Var references they may contain. (There cannot be cross-references
2616+
* between MULTIEXPR SubPlans, so we needn't worry about their order.)
2617+
*/
2618+
foreach(lc,info->multiexpr_subplans)
2619+
{
2620+
SubPlan*subplan= (SubPlan*)lfirst(lc);
2621+
SubPlanState*sstate;
2622+
2623+
Assert(subplan->subLinkType==MULTIEXPR_SUBLINK);
2624+
2625+
/* This should match what ExecInitExprRec does for other SubPlans: */
2626+
2627+
if (!state->parent)
2628+
elog(ERROR,"SubPlan found with no parent plan");
2629+
2630+
sstate=ExecInitSubPlan(subplan,state->parent);
2631+
2632+
/* add SubPlanState nodes to state->parent->subPlan */
2633+
state->parent->subPlan=lappend(state->parent->subPlan,
2634+
sstate);
2635+
2636+
scratch.opcode=EEOP_SUBPLAN;
2637+
scratch.d.subplan.sstate=sstate;
2638+
2639+
/* The result can be ignored, but we better put it somewhere */
2640+
scratch.resvalue=&state->resvalue;
2641+
scratch.resnull=&state->resnull;
2642+
2643+
ExprEvalPushStep(state,&scratch);
2644+
}
26052645
}
26062646

26072647
/*
2608-
*get_last_attnums_walker: expression walker forExecInitExprSlots
2648+
*expr_setup_walker: expression walker forExecCreateExprSetupSteps
26092649
*/
26102650
staticbool
2611-
get_last_attnums_walker(Node*node,LastAttnumInfo*info)
2651+
expr_setup_walker(Node*node,ExprSetupInfo*info)
26122652
{
26132653
if (node==NULL)
26142654
return false;
@@ -2636,6 +2676,16 @@ get_last_attnums_walker(Node *node, LastAttnumInfo *info)
26362676
return false;
26372677
}
26382678

2679+
/* Collect all MULTIEXPR SubPlans, too */
2680+
if (IsA(node,SubPlan))
2681+
{
2682+
SubPlan*subplan= (SubPlan*)node;
2683+
2684+
if (subplan->subLinkType==MULTIEXPR_SUBLINK)
2685+
info->multiexpr_subplans=lappend(info->multiexpr_subplans,
2686+
subplan);
2687+
}
2688+
26392689
/*
26402690
* Don't examine the arguments or filters of Aggrefs or WindowFuncs,
26412691
* because those do not represent expressions to be evaluated within the
@@ -2648,7 +2698,7 @@ get_last_attnums_walker(Node *node, LastAttnumInfo *info)
26482698
return false;
26492699
if (IsA(node,GroupingFunc))
26502700
return false;
2651-
returnexpression_tree_walker(node,get_last_attnums_walker,
2701+
returnexpression_tree_walker(node,expr_setup_walker,
26522702
(void*)info);
26532703
}
26542704

@@ -3267,7 +3317,7 @@ ExecBuildAggTrans(AggState *aggstate, AggStatePerPhase phase,
32673317
PlanState*parent=&aggstate->ss.ps;
32683318
ExprEvalStepscratch= {0};
32693319
boolisCombine=DO_AGGSPLIT_COMBINE(aggstate->aggsplit);
3270-
LastAttnumInfodeform= {0,0,0};
3320+
ExprSetupInfodeform= {0,0,0,NIL};
32713321

32723322
state->expr= (Expr*)aggstate;
32733323
state->parent=parent;
@@ -3283,18 +3333,18 @@ ExecBuildAggTrans(AggState *aggstate, AggStatePerPhase phase,
32833333
{
32843334
AggStatePerTranspertrans=&aggstate->pertrans[transno];
32853335

3286-
get_last_attnums_walker((Node*)pertrans->aggref->aggdirectargs,
3287-
&deform);
3288-
get_last_attnums_walker((Node*)pertrans->aggref->args,
3289-
&deform);
3290-
get_last_attnums_walker((Node*)pertrans->aggref->aggorder,
3291-
&deform);
3292-
get_last_attnums_walker((Node*)pertrans->aggref->aggdistinct,
3293-
&deform);
3294-
get_last_attnums_walker((Node*)pertrans->aggref->aggfilter,
3295-
&deform);
3336+
expr_setup_walker((Node*)pertrans->aggref->aggdirectargs,
3337+
&deform);
3338+
expr_setup_walker((Node*)pertrans->aggref->args,
3339+
&deform);
3340+
expr_setup_walker((Node*)pertrans->aggref->aggorder,
3341+
&deform);
3342+
expr_setup_walker((Node*)pertrans->aggref->aggdistinct,
3343+
&deform);
3344+
expr_setup_walker((Node*)pertrans->aggref->aggfilter,
3345+
&deform);
32963346
}
3297-
ExecPushExprSlots(state,&deform);
3347+
ExecPushExprSetupSteps(state,&deform);
32983348

32993349
/*
33003350
* Emit instructions for each transition value / grouping set combination.

0 commit comments

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