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

Commit4729d1e

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 parent89f5eb2 commit4729d1e

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
@@ -2441,7 +2441,7 @@ ExecBuildAuxRowMark(ExecRowMark *erm, List *targetlist)
24412441
*relation - table containing tuple
24422442
*rti - rangetable index of table containing tuple
24432443
*inputslot - tuple for processing - this can be the slot from
2444-
*EvalPlanQualSlot(), forthe increased efficiency.
2444+
*EvalPlanQualSlot() forthis rel, for increased efficiency.
24452445
*
24462446
* This tests whether the tuple in inputslot still matches the relevant
24472447
* quals. For that result to be useful, typically the input tuple has to be
@@ -2475,6 +2475,14 @@ EvalPlanQual(EPQState *epqstate, Relation relation,
24752475
if (testslot!=inputslot)
24762476
ExecCopySlot(testslot,inputslot);
24772477

2478+
/*
2479+
* Mark that an EPQ tuple is available for this relation. (If there is
2480+
* more than one result relation, the others remain marked as having no
2481+
* tuple available.)
2482+
*/
2483+
epqstate->relsubs_done[rti-1]= false;
2484+
epqstate->epqExtra->relsubs_blocked[rti-1]= false;
2485+
24782486
/*
24792487
* Run the EPQ query. We assume it will return at most one tuple.
24802488
*/
@@ -2491,11 +2499,12 @@ EvalPlanQual(EPQState *epqstate, Relation relation,
24912499
ExecMaterializeSlot(slot);
24922500

24932501
/*
2494-
* Clear out the test tuple. This is needed in case the EPQ query is
2495-
* re-used to test a tuple for a different relation. (Not clear that can
2496-
*really happen, but let's be safe.)
2502+
* Clear out the test tuple, and mark that no tuple is available here.
2503+
*This is needed in case the EPQ state isre-used to test a tuple for a
2504+
*different target relation.
24972505
*/
24982506
ExecClearTuple(testslot);
2507+
epqstate->epqExtra->relsubs_blocked[rti-1]= true;
24992508

25002509
returnslot;
25012510
}
@@ -2510,12 +2519,34 @@ EvalPlanQual(EPQState *epqstate, Relation relation,
25102519
void
25112520
EvalPlanQualInit(EPQState*epqstate,EState*parentestate,
25122521
Plan*subplan,List*auxrowmarks,intepqParam)
2522+
{
2523+
EvalPlanQualInitExt(epqstate,parentestate,
2524+
subplan,auxrowmarks,epqParam,NIL);
2525+
}
2526+
2527+
/*
2528+
* EvalPlanQualInitExt -- same, but allow specification of resultRelations
2529+
*
2530+
* If the caller intends to use EvalPlanQual(), resultRelations should be
2531+
* a list of RT indexes of potential target relations for EvalPlanQual(),
2532+
* and we will arrange that the other listed relations don't return any
2533+
* tuple during an EvalPlanQual() call. Otherwise resultRelations
2534+
* should be NIL.
2535+
*/
2536+
void
2537+
EvalPlanQualInitExt(EPQState*epqstate,EState*parentestate,
2538+
Plan*subplan,List*auxrowmarks,
2539+
intepqParam,List*resultRelations)
25132540
{
25142541
Indexrtsize=parentestate->es_range_table_size;
25152542

2543+
/* create some extra space to avoid ABI break */
2544+
epqstate->epqExtra= (EPQStateExtra*)palloc(sizeof(EPQStateExtra));
2545+
25162546
/* initialize data not changing over EPQState's lifetime */
25172547
epqstate->parentestate=parentestate;
25182548
epqstate->epqParam=epqParam;
2549+
epqstate->epqExtra->resultRelations=resultRelations;
25192550

25202551
/*
25212552
* Allocate space to reference a slot for each potential rti - do so now
@@ -2524,7 +2555,7 @@ EvalPlanQualInit(EPQState *epqstate, EState *parentestate,
25242555
* that *may* need EPQ later, without forcing the overhead of
25252556
* EvalPlanQualBegin().
25262557
*/
2527-
epqstate->tuple_table=NIL;
2558+
epqstate->epqExtra->tuple_table=NIL;
25282559
epqstate->relsubs_slot= (TupleTableSlot**)
25292560
palloc0(rtsize*sizeof(TupleTableSlot*));
25302561

@@ -2538,6 +2569,7 @@ EvalPlanQualInit(EPQState *epqstate, EState *parentestate,
25382569
epqstate->recheckplanstate=NULL;
25392570
epqstate->relsubs_rowmark=NULL;
25402571
epqstate->relsubs_done=NULL;
2572+
epqstate->epqExtra->relsubs_blocked=NULL;
25412573
}
25422574

25432575
/*
@@ -2578,7 +2610,7 @@ EvalPlanQualSlot(EPQState *epqstate,
25782610
MemoryContextoldcontext;
25792611

25802612
oldcontext=MemoryContextSwitchTo(epqstate->parentestate->es_query_cxt);
2581-
*slot=table_slot_create(relation,&epqstate->tuple_table);
2613+
*slot=table_slot_create(relation,&epqstate->epqExtra->tuple_table);
25822614
MemoryContextSwitchTo(oldcontext);
25832615
}
25842616

@@ -2735,7 +2767,13 @@ EvalPlanQualBegin(EPQState *epqstate)
27352767
Indexrtsize=parentestate->es_range_table_size;
27362768
PlanState*rcplanstate=epqstate->recheckplanstate;
27372769

2738-
MemSet(epqstate->relsubs_done,0,rtsize*sizeof(bool));
2770+
/*
2771+
* Reset the relsubs_done[] flags to equal relsubs_blocked[], so that
2772+
* the EPQ run will never attempt to fetch tuples from blocked target
2773+
* relations.
2774+
*/
2775+
memcpy(epqstate->relsubs_done,epqstate->epqExtra->relsubs_blocked,
2776+
rtsize*sizeof(bool));
27392777

27402778
/* Recopy current values of parent parameters */
27412779
if (parentestate->es_plannedstmt->paramExecTypes!=NIL)
@@ -2902,10 +2940,22 @@ EvalPlanQualStart(EPQState *epqstate, Plan *planTree)
29022940
}
29032941

29042942
/*
2905-
* Initialize per-relation EPQ tuple states to not-fetched.
2943+
* Initialize per-relation EPQ tuple states. Result relations, if any,
2944+
* get marked as blocked; others as not-fetched.
29062945
*/
2907-
epqstate->relsubs_done= (bool*)
2908-
palloc0(rtsize*sizeof(bool));
2946+
epqstate->relsubs_done=palloc_array(bool,rtsize);
2947+
epqstate->epqExtra->relsubs_blocked=palloc0_array(bool,rtsize);
2948+
2949+
foreach(l,epqstate->epqExtra->resultRelations)
2950+
{
2951+
intrtindex=lfirst_int(l);
2952+
2953+
Assert(rtindex>0&&rtindex <=rtsize);
2954+
epqstate->epqExtra->relsubs_blocked[rtindex-1]= true;
2955+
}
2956+
2957+
memcpy(epqstate->relsubs_done,epqstate->epqExtra->relsubs_blocked,
2958+
rtsize*sizeof(bool));
29092959

29102960
/*
29112961
* Initialize the private state information for all the nodes in the part
@@ -2942,12 +2992,12 @@ EvalPlanQualEnd(EPQState *epqstate)
29422992
* We may have a tuple table, even if EPQ wasn't started, because we allow
29432993
* use of EvalPlanQualSlot() without calling EvalPlanQualBegin().
29442994
*/
2945-
if (epqstate->tuple_table!=NIL)
2995+
if (epqstate->epqExtra->tuple_table!=NIL)
29462996
{
29472997
memset(epqstate->relsubs_slot,0,
29482998
rtsize*sizeof(TupleTableSlot*));
2949-
ExecResetTupleTable(epqstate->tuple_table, true);
2950-
epqstate->tuple_table=NIL;
2999+
ExecResetTupleTable(epqstate->epqExtra->tuple_table, true);
3000+
epqstate->epqExtra->tuple_table=NIL;
29513001
}
29523002

29533003
/* EPQ wasn't started, nothing further to do */
@@ -2981,4 +3031,5 @@ EvalPlanQualEnd(EPQState *epqstate)
29813031
epqstate->recheckplanstate=NULL;
29823032
epqstate->relsubs_rowmark=NULL;
29833033
epqstate->relsubs_done=NULL;
3034+
epqstate->epqExtra->relsubs_blocked=NULL;
29843035
}

‎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
@@ -3966,7 +3966,8 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
39663966
}
39673967

39683968
/* set up epqstate with dummy subplan data for the moment */
3969-
EvalPlanQualInit(&mtstate->mt_epqstate,estate,NULL,NIL,node->epqParam);
3969+
EvalPlanQualInitExt(&mtstate->mt_epqstate,estate,NULL,NIL,
3970+
node->epqParam,node->resultRelations);
39703971
mtstate->fireBSTriggers= true;
39713972

39723973
/*

‎src/include/executor/executor.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,9 @@ extern TupleTableSlot *EvalPlanQual(EPQState *epqstate, Relation relation,
221221
Indexrti,TupleTableSlot*testslot);
222222
externvoidEvalPlanQualInit(EPQState*epqstate,EState*parentestate,
223223
Plan*subplan,List*auxrowmarks,intepqParam);
224+
externvoidEvalPlanQualInitExt(EPQState*epqstate,EState*parentestate,
225+
Plan*subplan,List*auxrowmarks,
226+
intepqParam,List*resultRelations);
224227
externvoidEvalPlanQualSetPlan(EPQState*epqstate,
225228
Plan*subplan,List*auxrowmarks);
226229
externTupleTableSlot*EvalPlanQualSlot(EPQState*epqstate,

‎src/include/nodes/execnodes.h

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1153,17 +1153,17 @@ typedef struct PlanState
11531153
*/
11541154
typedefstructEPQState
11551155
{
1156-
/* Initialized at EvalPlanQualInit() time: */
1157-
1156+
/* These are initialized by EvalPlanQualInit() and do not change later: */
11581157
EState*parentestate;/* main query's EState */
11591158
intepqParam;/* ID of Param to force scan node re-eval */
1159+
structEPQStateExtra*epqExtra;/* extension pointer to avoid ABI break */
11601160

11611161
/*
1162-
* Tuples to be substituted by scan nodes. They need to set up, before
1163-
* calling EvalPlanQual()/EvalPlanQualNext(), into the slot returned by
1164-
* EvalPlanQualSlot(scanrelid). The array is indexed by scanrelid - 1.
1162+
* relsubs_slot[scanrelid - 1] holds the EPQ test tuple to be returned by
1163+
* the scan node for the scanrelid'th RT index, in place of performing an
1164+
* actual table scan. Callers should use EvalPlanQualSlot() to fetch
1165+
* these slots.
11651166
*/
1166-
List*tuple_table;/* tuple table for relsubs_slot */
11671167
TupleTableSlot**relsubs_slot;
11681168

11691169
/*
@@ -1195,15 +1195,35 @@ typedef struct EPQState
11951195
ExecAuxRowMark**relsubs_rowmark;
11961196

11971197
/*
1198-
* True if a relation's EPQ tuple has been fetched for relation, indexed
1199-
* by scanrelid - 1.
1198+
* relsubs_done[scanrelid - 1] is true if there is no EPQ tuple for this
1199+
* target relation or it has already been fetched in the current scan of
1200+
* this target relation within the current EvalPlanQual test.
12001201
*/
12011202
bool*relsubs_done;
12021203

12031204
PlanState*recheckplanstate;/* EPQ specific exec nodes, for ->plan */
12041205
}EPQState;
12051206

12061207

1208+
/*
1209+
* To avoid an ABI-breaking change in the size of EPQState in back branches,
1210+
* we create one of these during EvalPlanQualInit.
1211+
*/
1212+
typedefstructEPQStateExtra
1213+
{
1214+
List*resultRelations;/* integer list of RT indexes, or NIL */
1215+
List*tuple_table;/* tuple table for relsubs_slot */
1216+
1217+
/*
1218+
* relsubs_blocked[scanrelid - 1] is true if there is no EPQ tuple for
1219+
* this target relation during the current EvalPlanQual test. We keep
1220+
* these flags set for all relids listed in resultRelations, but
1221+
* transiently clear the one for the relation whose tuple is actually
1222+
* passed to EvalPlanQual().
1223+
*/
1224+
bool*relsubs_blocked;
1225+
}EPQStateExtra;
1226+
12071227
/* ----------------
12081228
* ResultState information
12091229
* ----------------

0 commit comments

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