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

Commitf8320cc

Browse files
committed
Fix misbehavior of EvalPlanQual checks with multiple result relations.
The idea of EvalPlanQual is that we replace the query's scan of theresult relation with a single injected tuple, and see if we get atuple out, thereby implying that the injected tuple still passes thequery quals. (In join cases, other relations in the query are stillscanned normally.) This logic was not updated when commit86dc900made it possible for a single DML query plan to have multiple resultrelations, when the query target relation has inheritance or partitionchildren. We replaced the output for the current result relationsuccessfully, but other result relations were still scanned normally;thus, if any other result relation contained a tuple satisfying thequals, we'd think the EPQ check passed, even if it did not pass forthe injected tuple itself. This would lead to update or deleteactions getting performed when they should have been skipped due toa conflicting concurrent update in READ COMMITTED isolation mode.Fix by blocking all sibling result relations from emitting tuplesduring an EvalPlanQual recheck. In the back branches, the fix iscomplicated a bit by the need to not change the size of structEPQState (else we'd have ABI-breaking changes in offsets instruct ModifyTableState). Like the back-patches of3f7836fand4b3e379, add a separately palloc'd struct to avoid that.The logic is the same as in HEAD otherwise.This is only a live bug back to v14 where86dc900 came in.However, I chose to back-patch the test cases further, on thegrounds that this whole area is none too well tested. I skippeddoing so in v11 though because none of the test applied cleanly,and it didn't quite seem worth extra work for a branch with onlysix months to live.Per report from Ante Krešić (via Aleksander Alekseev)Discussion:https://postgr.es/m/CAJ7c6TMBTN3rcz4=AjYhLPD_w3FFT0Wq_C15jxCDn8U4tZnH1g@mail.gmail.com
1 parent4cdda71 commitf8320cc

File tree

7 files changed

+240
-41
lines changed

7 files changed

+240
-41
lines changed

‎src/backend/executor/execMain.c

Lines changed: 64 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2343,7 +2343,7 @@ ExecBuildAuxRowMark(ExecRowMark *erm, List *targetlist)
23432343
*relation - table containing tuple
23442344
*rti - rangetable index of table containing tuple
23452345
*inputslot - tuple for processing - this can be the slot from
2346-
*EvalPlanQualSlot(), forthe increased efficiency.
2346+
*EvalPlanQualSlot() forthis rel, for increased efficiency.
23472347
*
23482348
* This tests whether the tuple in inputslot still matches the relevant
23492349
* quals. For that result to be useful, typically the input tuple has to be
@@ -2377,6 +2377,14 @@ EvalPlanQual(EPQState *epqstate, Relation relation,
23772377
if (testslot!=inputslot)
23782378
ExecCopySlot(testslot,inputslot);
23792379

2380+
/*
2381+
* Mark that an EPQ tuple is available for this relation. (If there is
2382+
* more than one result relation, the others remain marked as having no
2383+
* tuple available.)
2384+
*/
2385+
epqstate->relsubs_done[rti-1]= false;
2386+
epqstate->epqExtra->relsubs_blocked[rti-1]= false;
2387+
23802388
/*
23812389
* Run the EPQ query. We assume it will return at most one tuple.
23822390
*/
@@ -2393,11 +2401,12 @@ EvalPlanQual(EPQState *epqstate, Relation relation,
23932401
ExecMaterializeSlot(slot);
23942402

23952403
/*
2396-
* Clear out the test tuple. This is needed in case the EPQ query is
2397-
* re-used to test a tuple for a different relation. (Not clear that can
2398-
*really happen, but let's be safe.)
2404+
* Clear out the test tuple, and mark that no tuple is available here.
2405+
*This is needed in case the EPQ state isre-used to test a tuple for a
2406+
*different target relation.
23992407
*/
24002408
ExecClearTuple(testslot);
2409+
epqstate->epqExtra->relsubs_blocked[rti-1]= true;
24012410

24022411
returnslot;
24032412
}
@@ -2412,12 +2421,34 @@ EvalPlanQual(EPQState *epqstate, Relation relation,
24122421
void
24132422
EvalPlanQualInit(EPQState*epqstate,EState*parentestate,
24142423
Plan*subplan,List*auxrowmarks,intepqParam)
2424+
{
2425+
EvalPlanQualInitExt(epqstate,parentestate,
2426+
subplan,auxrowmarks,epqParam,NIL);
2427+
}
2428+
2429+
/*
2430+
* EvalPlanQualInitExt -- same, but allow specification of resultRelations
2431+
*
2432+
* If the caller intends to use EvalPlanQual(), resultRelations should be
2433+
* a list of RT indexes of potential target relations for EvalPlanQual(),
2434+
* and we will arrange that the other listed relations don't return any
2435+
* tuple during an EvalPlanQual() call. Otherwise resultRelations
2436+
* should be NIL.
2437+
*/
2438+
void
2439+
EvalPlanQualInitExt(EPQState*epqstate,EState*parentestate,
2440+
Plan*subplan,List*auxrowmarks,
2441+
intepqParam,List*resultRelations)
24152442
{
24162443
Indexrtsize=parentestate->es_range_table_size;
24172444

2445+
/* create some extra space to avoid ABI break */
2446+
epqstate->epqExtra= (EPQStateExtra*)palloc(sizeof(EPQStateExtra));
2447+
24182448
/* initialize data not changing over EPQState's lifetime */
24192449
epqstate->parentestate=parentestate;
24202450
epqstate->epqParam=epqParam;
2451+
epqstate->epqExtra->resultRelations=resultRelations;
24212452

24222453
/*
24232454
* Allocate space to reference a slot for each potential rti - do so now
@@ -2426,7 +2457,7 @@ EvalPlanQualInit(EPQState *epqstate, EState *parentestate,
24262457
* that *may* need EPQ later, without forcing the overhead of
24272458
* EvalPlanQualBegin().
24282459
*/
2429-
epqstate->tuple_table=NIL;
2460+
epqstate->epqExtra->tuple_table=NIL;
24302461
epqstate->relsubs_slot= (TupleTableSlot**)
24312462
palloc0(rtsize*sizeof(TupleTableSlot*));
24322463

@@ -2440,6 +2471,7 @@ EvalPlanQualInit(EPQState *epqstate, EState *parentestate,
24402471
epqstate->recheckplanstate=NULL;
24412472
epqstate->relsubs_rowmark=NULL;
24422473
epqstate->relsubs_done=NULL;
2474+
epqstate->epqExtra->relsubs_blocked=NULL;
24432475
}
24442476

24452477
/*
@@ -2480,7 +2512,7 @@ EvalPlanQualSlot(EPQState *epqstate,
24802512
MemoryContextoldcontext;
24812513

24822514
oldcontext=MemoryContextSwitchTo(epqstate->parentestate->es_query_cxt);
2483-
*slot=table_slot_create(relation,&epqstate->tuple_table);
2515+
*slot=table_slot_create(relation,&epqstate->epqExtra->tuple_table);
24842516
MemoryContextSwitchTo(oldcontext);
24852517
}
24862518

@@ -2637,7 +2669,13 @@ EvalPlanQualBegin(EPQState *epqstate)
26372669
Indexrtsize=parentestate->es_range_table_size;
26382670
PlanState*rcplanstate=epqstate->recheckplanstate;
26392671

2640-
MemSet(epqstate->relsubs_done,0,rtsize*sizeof(bool));
2672+
/*
2673+
* Reset the relsubs_done[] flags to equal relsubs_blocked[], so that
2674+
* the EPQ run will never attempt to fetch tuples from blocked target
2675+
* relations.
2676+
*/
2677+
memcpy(epqstate->relsubs_done,epqstate->epqExtra->relsubs_blocked,
2678+
rtsize*sizeof(bool));
26412679

26422680
/* Recopy current values of parent parameters */
26432681
if (parentestate->es_plannedstmt->paramExecTypes!=NIL)
@@ -2804,10 +2842,22 @@ EvalPlanQualStart(EPQState *epqstate, Plan *planTree)
28042842
}
28052843

28062844
/*
2807-
* Initialize per-relation EPQ tuple states to not-fetched.
2845+
* Initialize per-relation EPQ tuple states. Result relations, if any,
2846+
* get marked as blocked; others as not-fetched.
28082847
*/
2809-
epqstate->relsubs_done= (bool*)
2810-
palloc0(rtsize*sizeof(bool));
2848+
epqstate->relsubs_done=palloc_array(bool,rtsize);
2849+
epqstate->epqExtra->relsubs_blocked=palloc0_array(bool,rtsize);
2850+
2851+
foreach(l,epqstate->epqExtra->resultRelations)
2852+
{
2853+
intrtindex=lfirst_int(l);
2854+
2855+
Assert(rtindex>0&&rtindex <=rtsize);
2856+
epqstate->epqExtra->relsubs_blocked[rtindex-1]= true;
2857+
}
2858+
2859+
memcpy(epqstate->relsubs_done,epqstate->epqExtra->relsubs_blocked,
2860+
rtsize*sizeof(bool));
28112861

28122862
/*
28132863
* Initialize the private state information for all the nodes in the part
@@ -2844,12 +2894,12 @@ EvalPlanQualEnd(EPQState *epqstate)
28442894
* We may have a tuple table, even if EPQ wasn't started, because we allow
28452895
* use of EvalPlanQualSlot() without calling EvalPlanQualBegin().
28462896
*/
2847-
if (epqstate->tuple_table!=NIL)
2897+
if (epqstate->epqExtra->tuple_table!=NIL)
28482898
{
28492899
memset(epqstate->relsubs_slot,0,
28502900
rtsize*sizeof(TupleTableSlot*));
2851-
ExecResetTupleTable(epqstate->tuple_table, true);
2852-
epqstate->tuple_table=NIL;
2901+
ExecResetTupleTable(epqstate->epqExtra->tuple_table, true);
2902+
epqstate->epqExtra->tuple_table=NIL;
28532903
}
28542904

28552905
/* EPQ wasn't started, nothing further to do */
@@ -2883,4 +2933,5 @@ EvalPlanQualEnd(EPQState *epqstate)
28832933
epqstate->recheckplanstate=NULL;
28842934
epqstate->relsubs_rowmark=NULL;
28852935
epqstate->relsubs_done=NULL;
2936+
epqstate->epqExtra->relsubs_blocked=NULL;
28862937
}

‎src/backend/executor/execScan.c

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -69,13 +69,12 @@ ExecScanFetch(ScanState *node,
6969
elseif (epqstate->relsubs_done[scanrelid-1])
7070
{
7171
/*
72-
* Return empty slot, aswe already performed an EPQsubstitution
73-
*for this relation.
72+
* Return empty slot, aseither there is no EPQtuple for this rel
73+
*or we already returned it.
7474
*/
7575

7676
TupleTableSlot*slot=node->ss_ScanTupleSlot;
7777

78-
/* Return empty slot, as we already returned a tuple */
7978
returnExecClearTuple(slot);
8079
}
8180
elseif (epqstate->relsubs_slot[scanrelid-1]!=NULL)
@@ -88,7 +87,7 @@ ExecScanFetch(ScanState *node,
8887

8988
Assert(epqstate->relsubs_rowmark[scanrelid-1]==NULL);
9089

91-
/* Mark to remember that we shouldn't returnmore */
90+
/* Mark to remember that we shouldn't returnit again */
9291
epqstate->relsubs_done[scanrelid-1]= true;
9392

9493
/* Return empty slot if we haven't got a test tuple */
@@ -306,14 +305,18 @@ ExecScanReScan(ScanState *node)
306305
*/
307306
ExecClearTuple(node->ss_ScanTupleSlot);
308307

309-
/* Rescan EvalPlanQual tuple if we're inside an EvalPlanQual recheck */
308+
/*
309+
* Rescan EvalPlanQual tuple(s) if we're inside an EvalPlanQual recheck.
310+
* But don't lose the "blocked" status of blocked target relations.
311+
*/
310312
if (estate->es_epq_active!=NULL)
311313
{
312314
EPQState*epqstate=estate->es_epq_active;
313315
Indexscanrelid= ((Scan*)node->ps.plan)->scanrelid;
314316

315317
if (scanrelid>0)
316-
epqstate->relsubs_done[scanrelid-1]= false;
318+
epqstate->relsubs_done[scanrelid-1]=
319+
epqstate->epqExtra->relsubs_blocked[scanrelid-1];
317320
else
318321
{
319322
Bitmapset*relids;
@@ -335,7 +338,8 @@ ExecScanReScan(ScanState *node)
335338
while ((rtindex=bms_next_member(relids,rtindex)) >=0)
336339
{
337340
Assert(rtindex>0);
338-
epqstate->relsubs_done[rtindex-1]= false;
341+
epqstate->relsubs_done[rtindex-1]=
342+
epqstate->epqExtra->relsubs_blocked[rtindex-1];
339343
}
340344
}
341345
}

‎src/backend/executor/nodeModifyTable.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2880,7 +2880,8 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
28802880
}
28812881

28822882
/* set up epqstate with dummy subplan data for the moment */
2883-
EvalPlanQualInit(&mtstate->mt_epqstate,estate,NULL,NIL,node->epqParam);
2883+
EvalPlanQualInitExt(&mtstate->mt_epqstate,estate,NULL,NIL,
2884+
node->epqParam,node->resultRelations);
28842885
mtstate->fireBSTriggers= true;
28852886

28862887
/*

‎src/include/executor/executor.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,9 @@ extern TupleTableSlot *EvalPlanQual(EPQState *epqstate, Relation relation,
219219
Indexrti,TupleTableSlot*testslot);
220220
externvoidEvalPlanQualInit(EPQState*epqstate,EState*parentestate,
221221
Plan*subplan,List*auxrowmarks,intepqParam);
222+
externvoidEvalPlanQualInitExt(EPQState*epqstate,EState*parentestate,
223+
Plan*subplan,List*auxrowmarks,
224+
intepqParam,List*resultRelations);
222225
externvoidEvalPlanQualSetPlan(EPQState*epqstate,
223226
Plan*subplan,List*auxrowmarks);
224227
externTupleTableSlot*EvalPlanQualSlot(EPQState*epqstate,

‎src/include/nodes/execnodes.h

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1121,17 +1121,17 @@ typedef struct PlanState
11211121
*/
11221122
typedefstructEPQState
11231123
{
1124-
/* Initialized at EvalPlanQualInit() time: */
1125-
1124+
/* These are initialized by EvalPlanQualInit() and do not change later: */
11261125
EState*parentestate;/* main query's EState */
11271126
intepqParam;/* ID of Param to force scan node re-eval */
1127+
structEPQStateExtra*epqExtra;/* extension pointer to avoid ABI break */
11281128

11291129
/*
1130-
* Tuples to be substituted by scan nodes. They need to set up, before
1131-
* calling EvalPlanQual()/EvalPlanQualNext(), into the slot returned by
1132-
* EvalPlanQualSlot(scanrelid). The array is indexed by scanrelid - 1.
1130+
* relsubs_slot[scanrelid - 1] holds the EPQ test tuple to be returned by
1131+
* the scan node for the scanrelid'th RT index, in place of performing an
1132+
* actual table scan. Callers should use EvalPlanQualSlot() to fetch
1133+
* these slots.
11331134
*/
1134-
List*tuple_table;/* tuple table for relsubs_slot */
11351135
TupleTableSlot**relsubs_slot;
11361136

11371137
/*
@@ -1163,15 +1163,35 @@ typedef struct EPQState
11631163
ExecAuxRowMark**relsubs_rowmark;
11641164

11651165
/*
1166-
* True if a relation's EPQ tuple has been fetched for relation, indexed
1167-
* by scanrelid - 1.
1166+
* relsubs_done[scanrelid - 1] is true if there is no EPQ tuple for this
1167+
* target relation or it has already been fetched in the current scan of
1168+
* this target relation within the current EvalPlanQual test.
11681169
*/
11691170
bool*relsubs_done;
11701171

11711172
PlanState*recheckplanstate;/* EPQ specific exec nodes, for ->plan */
11721173
}EPQState;
11731174

11741175

1176+
/*
1177+
* To avoid an ABI-breaking change in the size of EPQState in back branches,
1178+
* we create one of these during EvalPlanQualInit.
1179+
*/
1180+
typedefstructEPQStateExtra
1181+
{
1182+
List*resultRelations;/* integer list of RT indexes, or NIL */
1183+
List*tuple_table;/* tuple table for relsubs_slot */
1184+
1185+
/*
1186+
* relsubs_blocked[scanrelid - 1] is true if there is no EPQ tuple for
1187+
* this target relation during the current EvalPlanQual test. We keep
1188+
* these flags set for all relids listed in resultRelations, but
1189+
* transiently clear the one for the relation whose tuple is actually
1190+
* passed to EvalPlanQual().
1191+
*/
1192+
bool*relsubs_blocked;
1193+
}EPQStateExtra;
1194+
11751195
/* ----------------
11761196
* ResultState information
11771197
* ----------------

0 commit comments

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