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

Commit87fed2a

Browse files
committed
Fix dangling pointer in EvalPlanQual machinery.
EvalPlanQualStart() supposed that it could re-use the relsubs_rowmarkand relsubs_done arrays from a prior instantiation. But since they areallocated in the es_query_cxt of the recheckestate, that's just wrong;EvalPlanQualEnd() will blow away that storage. Therefore we were usingstorage that could have been reallocated to something else, causing allsorts of havoc.I think this was modeled on the old code's handling of es_epqTupleSlot,but since the code was anyway clearing the arrays at re-use, there'sclearly no expectation of importing any outside state. So it's justa dubious savings of a couple of pallocs, which is negligible comparedto setting up a new planstate tree. Therefore, just allocate thearrays always. (I moved the allocations slightly for readability.)In principle this bug could cause a problem whenever EPQ rechecks areneeded in more than one target table of a ModifyTable plan node.In practice it seems not quite so easy to trigger as that; I couldn'treadily duplicate a crash with a partitioned target table, for instance.That's probably down to incidental choices about when to free orreallocate stuff. The added isolation test case does seem to reliablyshow an assertion failure, though.Per report from Oleksii Kliukin. Back-patch to v12 where the bug wasintroduced (evidently by commit3fb307b).Discussion:https://postgr.es/m/EEF05F66-2871-4786-992B-5F45C92FEE2E@hintbits.com
1 parentf9d0be2 commit87fed2a

File tree

3 files changed

+22
-23
lines changed

3 files changed

+22
-23
lines changed

‎src/backend/executor/execMain.c

Lines changed: 11 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2891,40 +2891,26 @@ EvalPlanQualStart(EPQState *epqstate, Plan *planTree)
28912891
subplanstate);
28922892
}
28932893

2894-
/*
2895-
* These arrays are reused across different plans set with
2896-
* EvalPlanQualSetPlan(), which is safe because they all use the same
2897-
* parent EState. Therefore we can reuse if already allocated.
2898-
*/
2899-
if (epqstate->relsubs_rowmark==NULL)
2900-
{
2901-
Assert(epqstate->relsubs_done==NULL);
2902-
epqstate->relsubs_rowmark= (ExecAuxRowMark**)
2903-
palloc0(rtsize*sizeof(ExecAuxRowMark*));
2904-
epqstate->relsubs_done= (bool*)
2905-
palloc0(rtsize*sizeof(bool));
2906-
}
2907-
else
2908-
{
2909-
Assert(epqstate->relsubs_done!=NULL);
2910-
memset(epqstate->relsubs_rowmark,0,
2911-
rtsize*sizeof(ExecAuxRowMark*));
2912-
memset(epqstate->relsubs_done,0,
2913-
rtsize*sizeof(bool));
2914-
}
2915-
29162894
/*
29172895
* Build an RTI indexed array of rowmarks, so that
29182896
* EvalPlanQualFetchRowMark() can efficiently access the to be fetched
29192897
* rowmark.
29202898
*/
2899+
epqstate->relsubs_rowmark= (ExecAuxRowMark**)
2900+
palloc0(rtsize*sizeof(ExecAuxRowMark*));
29212901
foreach(l,epqstate->arowMarks)
29222902
{
29232903
ExecAuxRowMark*earm= (ExecAuxRowMark*)lfirst(l);
29242904

29252905
epqstate->relsubs_rowmark[earm->rowmark->rti-1]=earm;
29262906
}
29272907

2908+
/*
2909+
* Initialize per-relation EPQ tuple states to not-fetched.
2910+
*/
2911+
epqstate->relsubs_done= (bool*)
2912+
palloc0(rtsize*sizeof(bool));
2913+
29282914
/*
29292915
* Initialize the private state information for all the nodes in the part
29302916
* of the plan tree we need to run. This opens files, allocates storage
@@ -2993,7 +2979,9 @@ EvalPlanQualEnd(EPQState *epqstate)
29932979
FreeExecutorState(estate);
29942980

29952981
/* Mark EPQState idle */
2982+
epqstate->origslot=NULL;
29962983
epqstate->recheckestate=NULL;
29972984
epqstate->recheckplanstate=NULL;
2998-
epqstate->origslot=NULL;
2985+
epqstate->relsubs_rowmark=NULL;
2986+
epqstate->relsubs_done=NULL;
29992987
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -673,6 +673,13 @@ a b c
673673
2 3 0
674674
step c2: COMMIT;
675675

676+
starting permutation: writep3a writep3b c1 c2
677+
step writep3a: UPDATE p SET b = -b WHERE c = 0;
678+
step writep3b: UPDATE p SET b = -b WHERE c = 0; <waiting ...>
679+
step c1: COMMIT;
680+
step writep3b: <... completed>
681+
step c2: COMMIT;
682+
676683
starting permutation: wx2 partiallock c2 c1 read
677684
step wx2: UPDATE accounts SET balance = balance + 450 WHERE accountid = 'checking' RETURNING balance;
678685
balance

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,10 +97,12 @@ step "upsert1"{
9797
# readp1/writep1/readp2 tests a bug where nodeLockRows did the wrong thing
9898
# when the first updated tuple was in a non-first child table.
9999
# writep2/returningp1 tests a memory allocation issue
100+
# writep3a/writep3b tests updates touching more than one table
100101

101102
step"readp1"{SELECTtableoid::regclass,ctid,*FROMpWHEREbIN (0,1)ANDc=0FORUPDATE; }
102103
step"writep1"{UPDATEpSETb=-1WHEREa=1ANDb=1ANDc=0; }
103104
step"writep2"{UPDATEpSETb=-bWHEREa=1ANDc=0; }
105+
step"writep3a"{UPDATEpSETb=-bWHEREc=0; }
104106
step"c1"{COMMIT; }
105107
step"r1"{ROLLBACK; }
106108

@@ -203,6 +205,7 @@ step "returningp1" {
203205
WITHuAS (UPDATEpSETb=bWHEREa>0RETURNING* )
204206
SELECT*FROMu;
205207
}
208+
step"writep3b"{UPDATEpSETb=-bWHEREc=0; }
206209
step"readforss"{
207210
SELECTta.idASta_id,ta.valueASta_value,
208211
(SELECTROW(tb.id,tb.value)
@@ -338,6 +341,7 @@ permutation "wx1" "delwctefail" "c1" "c2" "read"
338341
permutation"upsert1""upsert2""c1""c2""read"
339342
permutation"readp1""writep1""readp2""c1""c2"
340343
permutation"writep2""returningp1""c1""c2"
344+
permutation"writep3a""writep3b""c1""c2"
341345
permutation"wx2""partiallock""c2""c1""read"
342346
permutation"wx2""lockwithvalues""c2""c1""read"
343347
permutation"wx2_ext""partiallock_ext""c2""c1""read_ext"

0 commit comments

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