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

Commit2a97a0a

Browse files
committed
Fix failure with initplans used conditionally during EvalPlanQual rechecks.
The EvalPlanQual machinery assumes that any initplans (that is,uncorrelated sub-selects) used during an EPQ recheck would have alreadybeen evaluated during the main query; this is implicit in the fact thatexecPlan pointers are not copied into the EPQ estate's es_param_exec_vals.But it's possible for that assumption to fail, if the initplan is onlyreached conditionally. For example, a sub-select inside a CASE expressioncould be reached during a recheck when it had not been previously, if theCASE test depends on a column that was just updated.This bug is old, appearing to date back to my rewrite of EvalPlanQual incommit9f2ee8f, but was not detected until Kyle Samson reported a case.To fix, force all not-yet-evaluated initplans used within the EPQ plansubtree to be evaluated at the start of the recheck, before entering theEPQ environment. This could be inefficient, if such an initplan isexpensive and goes unused again during the recheck --- but that's pilingone layer of improbability atop another. It doesn't seem worth addingmore complexity to prevent that, at least not in the back branches.It was convenient to use the new-in-v11 ExecEvalParamExecParams functionto implement this, but I didn't like either its name or the specifics ofits API, so revise that.Back-patch all the way. Rather than rewrite the patch to avoid dependingon bms_next_member() in the oldest branches, I chose to back-patch thatfunction into 9.4 and 9.3. (This isn't the first time back-patches haveneeded that, and it exhausted my patience.) I also chose to back-patchsome test cases added by commits71404af and342a1ff into 9.4 and 9.3,so that the 9.x versions of eval-plan-qual.spec are all the same.Andrew Gierth diagnosed the problem and contributed the added test cases,though the actual code changes are by me.Discussion:https://postgr.es/m/A033A40A-B234-4324-BE37-272279F7B627@tripadvisor.com
1 parent568b4e1 commit2a97a0a

File tree

5 files changed

+131
-4
lines changed

5 files changed

+131
-4
lines changed

‎src/backend/executor/execMain.c

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
#include"commands/matview.h"
4646
#include"commands/trigger.h"
4747
#include"executor/execdebug.h"
48+
#include"executor/nodeSubplan.h"
4849
#include"foreign/fdwapi.h"
4950
#include"mb/pg_wchar.h"
5051
#include"miscadmin.h"
@@ -1572,8 +1573,8 @@ ExecutePlan(EState *estate,
15721573
if (TupIsNull(slot))
15731574
{
15741575
/*
1575-
* If we know we won't need to back up, we can release
1576-
*resourcesat this point.
1576+
* If we know we won't need to back up, we can release resources
1577+
* at this point.
15771578
*/
15781579
if (!(estate->es_top_eflags&EXEC_FLAG_BACKWARD))
15791580
(void)ExecShutdownNode(planstate);
@@ -2718,7 +2719,17 @@ EvalPlanQualBegin(EPQState *epqstate, EState *parentestate)
27182719
/* Recopy current values of parent parameters */
27192720
if (parentestate->es_plannedstmt->nParamExec>0)
27202721
{
2721-
inti=parentestate->es_plannedstmt->nParamExec;
2722+
inti;
2723+
2724+
/*
2725+
* Force evaluation of any InitPlan outputs that could be needed
2726+
* by the subplan, just in case they got reset since
2727+
* EvalPlanQualStart (see comments therein).
2728+
*/
2729+
ExecSetParamPlanMulti(planstate->plan->extParam,
2730+
GetPerTupleExprContext(parentestate));
2731+
2732+
i=parentestate->es_plannedstmt->nParamExec;
27222733

27232734
while (--i >=0)
27242735
{
@@ -2808,10 +2819,34 @@ EvalPlanQualStart(EPQState *epqstate, EState *parentestate, Plan *planTree)
28082819
estate->es_param_list_info=parentestate->es_param_list_info;
28092820
if (parentestate->es_plannedstmt->nParamExec>0)
28102821
{
2811-
inti=parentestate->es_plannedstmt->nParamExec;
2822+
inti;
2823+
2824+
/*
2825+
* Force evaluation of any InitPlan outputs that could be needed by
2826+
* the subplan. (With more complexity, maybe we could postpone this
2827+
* till the subplan actually demands them, but it doesn't seem worth
2828+
* the trouble; this is a corner case already, since usually the
2829+
* InitPlans would have been evaluated before reaching EvalPlanQual.)
2830+
*
2831+
* This will not touch output params of InitPlans that occur somewhere
2832+
* within the subplan tree, only those that are attached to the
2833+
* ModifyTable node or above it and are referenced within the subplan.
2834+
* That's OK though, because the planner would only attach such
2835+
* InitPlans to a lower-level SubqueryScan node, and EPQ execution
2836+
* will not descend into a SubqueryScan.
2837+
*
2838+
* The EState's per-output-tuple econtext is sufficiently short-lived
2839+
* for this, since it should get reset before there is any chance of
2840+
* doing EvalPlanQual again.
2841+
*/
2842+
ExecSetParamPlanMulti(planTree->extParam,
2843+
GetPerTupleExprContext(parentestate));
28122844

2845+
/* now make the internal param workspace ... */
2846+
i=parentestate->es_plannedstmt->nParamExec;
28132847
estate->es_param_exec_vals= (ParamExecData*)
28142848
palloc0(i*sizeof(ParamExecData));
2849+
/* ... and copy down all values, whether really needed or not */
28152850
while (--i >=0)
28162851
{
28172852
/* copy value if any, but not execPlan link */

‎src/backend/executor/nodeSubplan.c

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -947,6 +947,17 @@ ExecInitSubPlan(SubPlan *subplan, PlanState *parent)
947947
* of initplans: we don't run the subplan until/unless we need its output.
948948
* Note that this routine MUST clear the execPlan fields of the plan's
949949
* output parameters after evaluating them!
950+
*
951+
* The results of this function are stored in the EState associated with the
952+
* ExprContext (particularly, its ecxt_param_exec_vals); any pass-by-ref
953+
* result Datums are allocated in the EState's per-query memory. The passed
954+
* econtext can be any ExprContext belonging to that EState; which one is
955+
* important only to the extent that the ExprContext's per-tuple memory
956+
* context is used to evaluate any parameters passed down to the subplan.
957+
* (Thus in principle, the shorter-lived the ExprContext the better, since
958+
* that data isn't needed after we return. In practice, because initplan
959+
* parameters are never more complex than Vars, Aggrefs, etc, evaluating them
960+
* currently never leaks any memory anyway.)
950961
* ----------------------------------------------------------------
951962
*/
952963
void
@@ -1134,6 +1145,37 @@ ExecSetParamPlan(SubPlanState *node, ExprContext *econtext)
11341145
estate->es_direction=dir;
11351146
}
11361147

1148+
/*
1149+
* ExecSetParamPlanMulti
1150+
*
1151+
* Apply ExecSetParamPlan to evaluate any not-yet-evaluated initplan output
1152+
* parameters whose ParamIDs are listed in "params". Any listed params that
1153+
* are not initplan outputs are ignored.
1154+
*
1155+
* As with ExecSetParamPlan, any ExprContext belonging to the current EState
1156+
* can be used, but in principle a shorter-lived ExprContext is better than a
1157+
* longer-lived one.
1158+
*/
1159+
void
1160+
ExecSetParamPlanMulti(constBitmapset*params,ExprContext*econtext)
1161+
{
1162+
intparamid;
1163+
1164+
paramid=-1;
1165+
while ((paramid=bms_next_member(params,paramid)) >=0)
1166+
{
1167+
ParamExecData*prm=&(econtext->ecxt_param_exec_vals[paramid]);
1168+
1169+
if (prm->execPlan!=NULL)
1170+
{
1171+
/* Parameter not evaluated yet, so go do it */
1172+
ExecSetParamPlan(prm->execPlan,econtext);
1173+
/* ExecSetParamPlan should have processed this param... */
1174+
Assert(prm->execPlan==NULL);
1175+
}
1176+
}
1177+
}
1178+
11371179
/*
11381180
* Mark an initplan as needing recalculation
11391181
*/

‎src/include/executor/nodeSubplan.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,6 @@ extern void ExecReScanSetParamPlan(SubPlanState *node, PlanState *parent);
2424

2525
externvoidExecSetParamPlan(SubPlanState*node,ExprContext*econtext);
2626

27+
externvoidExecSetParamPlanMulti(constBitmapset*params,ExprContext*econtext);
28+
2729
#endif/* NODESUBPLAN_H */

‎src/test/isolation/expected/eval-plan-qual.out

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,37 @@ ta_id ta_value tb_row
164164
1 newTableAValue (1,tableBValue)
165165
step c2: COMMIT;
166166

167+
starting permutation: updateforcip updateforcip2 c1 c2 read_a
168+
step updateforcip:
169+
UPDATE table_a SET value = NULL WHERE id = 1;
170+
171+
step updateforcip2:
172+
UPDATE table_a SET value = COALESCE(value, (SELECT text 'newValue')) WHERE id = 1;
173+
<waiting ...>
174+
step c1: COMMIT;
175+
step updateforcip2: <... completed>
176+
step c2: COMMIT;
177+
step read_a: SELECT * FROM table_a ORDER BY id;
178+
id value
179+
180+
1 newValue
181+
182+
starting permutation: updateforcip updateforcip3 c1 c2 read_a
183+
step updateforcip:
184+
UPDATE table_a SET value = NULL WHERE id = 1;
185+
186+
step updateforcip3:
187+
WITH d(val) AS (SELECT text 'newValue' FROM generate_series(1,1))
188+
UPDATE table_a SET value = COALESCE(value, (SELECT val FROM d)) WHERE id = 1;
189+
<waiting ...>
190+
step c1: COMMIT;
191+
step updateforcip3: <... completed>
192+
step c2: COMMIT;
193+
step read_a: SELECT * FROM table_a ORDER BY id;
194+
id value
195+
196+
1 newValue
197+
167198
starting permutation: wrtwcte readwcte c1 c2
168199
step wrtwcte: UPDATE table_a SET value = 'tableAValue2' WHERE id = 1;
169200
step readwcte:

‎src/test/isolation/specs/eval-plan-qual.spec

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,13 @@ step "updateforss"{
7878
UPDATEtable_bSETvalue='newTableBValue'WHEREid=1;
7979
}
8080

81+
# these tests exercise EvalPlanQual with conditional InitPlans which
82+
# have not been executed prior to the EPQ
83+
84+
step"updateforcip"{
85+
UPDATEtable_aSETvalue=NULLWHEREid=1;
86+
}
87+
8188

8289
session"s2"
8390
setup{BEGINISOLATIONLEVELREADCOMMITTED; }
@@ -103,12 +110,20 @@ step "readforss"{
103110
FROMtable_ata
104111
WHEREta.id=1FORUPDATEOFta;
105112
}
113+
step"updateforcip2"{
114+
UPDATEtable_aSETvalue=COALESCE(value, (SELECTtext'newValue'))WHEREid=1;
115+
}
116+
step"updateforcip3"{
117+
WITHd(val)AS (SELECTtext'newValue'FROMgenerate_series(1,1))
118+
UPDATEtable_aSETvalue=COALESCE(value, (SELECTvalFROMd))WHEREid=1;
119+
}
106120
step"wrtwcte"{UPDATEtable_aSETvalue='tableAValue2'WHEREid=1; }
107121
step"c2"{COMMIT; }
108122

109123
session"s3"
110124
setup{BEGINISOLATIONLEVELREADCOMMITTED; }
111125
step"read"{SELECT*FROMaccountsORDERBYaccountid; }
126+
step"read_a"{SELECT*FROMtable_aORDERBYid; }
112127

113128
# this test exercises EvalPlanQual with a CTE, cf bug #14328
114129
step"readwcte"{
@@ -142,5 +157,7 @@ permutation "writep2" "returningp1" "c1" "c2"
142157
permutation"wx2""partiallock""c2""c1""read"
143158
permutation"wx2""lockwithvalues""c2""c1""read"
144159
permutation"updateforss""readforss""c1""c2"
160+
permutation"updateforcip""updateforcip2""c1""c2""read_a"
161+
permutation"updateforcip""updateforcip3""c1""c2""read_a"
145162
permutation"wrtwcte""readwcte""c1""c2"
146163
permutation"wrtwcte""multireadwcte""c1""c2"

0 commit comments

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