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

Commit01d9676

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 parent30012a0 commit01d9676

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
@@ -2887,40 +2887,26 @@ EvalPlanQualStart(EPQState *epqstate, Plan *planTree)
28872887
subplanstate);
28882888
}
28892889

2890-
/*
2891-
* These arrays are reused across different plans set with
2892-
* EvalPlanQualSetPlan(), which is safe because they all use the same
2893-
* parent EState. Therefore we can reuse if already allocated.
2894-
*/
2895-
if (epqstate->relsubs_rowmark==NULL)
2896-
{
2897-
Assert(epqstate->relsubs_done==NULL);
2898-
epqstate->relsubs_rowmark= (ExecAuxRowMark**)
2899-
palloc0(rtsize*sizeof(ExecAuxRowMark*));
2900-
epqstate->relsubs_done= (bool*)
2901-
palloc0(rtsize*sizeof(bool));
2902-
}
2903-
else
2904-
{
2905-
Assert(epqstate->relsubs_done!=NULL);
2906-
memset(epqstate->relsubs_rowmark,0,
2907-
rtsize*sizeof(ExecAuxRowMark*));
2908-
memset(epqstate->relsubs_done,0,
2909-
rtsize*sizeof(bool));
2910-
}
2911-
29122890
/*
29132891
* Build an RTI indexed array of rowmarks, so that
29142892
* EvalPlanQualFetchRowMark() can efficiently access the to be fetched
29152893
* rowmark.
29162894
*/
2895+
epqstate->relsubs_rowmark= (ExecAuxRowMark**)
2896+
palloc0(rtsize*sizeof(ExecAuxRowMark*));
29172897
foreach(l,epqstate->arowMarks)
29182898
{
29192899
ExecAuxRowMark*earm= (ExecAuxRowMark*)lfirst(l);
29202900

29212901
epqstate->relsubs_rowmark[earm->rowmark->rti-1]=earm;
29222902
}
29232903

2904+
/*
2905+
* Initialize per-relation EPQ tuple states to not-fetched.
2906+
*/
2907+
epqstate->relsubs_done= (bool*)
2908+
palloc0(rtsize*sizeof(bool));
2909+
29242910
/*
29252911
* Initialize the private state information for all the nodes in the part
29262912
* of the plan tree we need to run. This opens files, allocates storage
@@ -2989,7 +2975,9 @@ EvalPlanQualEnd(EPQState *epqstate)
29892975
FreeExecutorState(estate);
29902976

29912977
/* Mark EPQState idle */
2978+
epqstate->origslot=NULL;
29922979
epqstate->recheckestate=NULL;
29932980
epqstate->recheckplanstate=NULL;
2994-
epqstate->origslot=NULL;
2981+
epqstate->relsubs_rowmark=NULL;
2982+
epqstate->relsubs_done=NULL;
29952983
}

‎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