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

Commit6868ed7

Browse files
committed
Throw error if expiring tuple is again updated or deleted.
This prevents surprising behavior when a FOR EACH ROW triggerBEFORE UPDATE or BEFORE DELETE directly or indirectly updates ordeletes the the old row. Prior to this patch the requested actionon the row could be silently ignored while all triggered actionsbased on the occurence of the requested action could be committed.One example of how this could happen is if the BEFORE DELETEtrigger for a "parent" row deleted "children" which had triggerfunctions to update summary or status data on the parent.This also prevents similar surprising problems if the query has avolatile function which updates a target row while it is alreadybeing updated.There are related issues present in FOR UPDATE cursors and READCOMMITTED queries which are not handled by this patch. Theseissues need further evalution to determine what change, if any, isneeded.Where the new error messages are generated, in most cases the bestfix will be to move code from the BEFORE trigger to an AFTERtrigger. Where this is not feasible, the trigger can avoid theerror by re-issuing the triggering statement and returning NULL.Documentation changes will be submitted in a separate patch.Kevin Grittner and Tom Lane with input from Florian Pflug andRobert Haas, based on problems encountered during conversion ofWisconsin Circuit Court trigger logic to plpgsql triggers.
1 parent17804fa commit6868ed7

File tree

8 files changed

+566
-98
lines changed

8 files changed

+566
-98
lines changed

‎src/backend/access/heap/heapam.c

Lines changed: 46 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -2352,27 +2352,26 @@ simple_heap_insert(Relation relation, HeapTuple tup)
23522352
*
23532353
*relation - table to be modified (caller must hold suitable lock)
23542354
*tid - TID of tuple to be deleted
2355-
*ctid - output parameter, used only for failure case (see below)
2356-
*update_xmax - output parameter, used only for failure case (see below)
23572355
*cid - delete command ID (used for visibility test, and stored into
23582356
*cmax if successful)
23592357
*crosscheck - if not InvalidSnapshot, also check tuple against this
23602358
*wait - true if should wait for any conflicting update to commit/abort
2359+
*hufd - output parameter, filled in failure cases (see below)
23612360
*
23622361
* Normal, successful return value is HeapTupleMayBeUpdated, which
23632362
* actually means we did delete it. Failure return codes are
23642363
* HeapTupleSelfUpdated, HeapTupleUpdated, or HeapTupleBeingUpdated
23652364
* (the last only possible if wait == false).
23662365
*
2367-
* In the failure cases, the routinereturnsthe tuple's t_ctid and t_xmax.
2368-
*If t_ctid isthesame as tid, the tuple was deleted; if different, the
2369-
*tuple was updated, and t_ctid is the location of the replacement tuple.
2370-
*(t_xmax is needed to verify that the replacement tuple matches.)
2366+
* In the failure cases, the routinefills *hufd withthe tuple's t_ctid,
2367+
*t_xmax, and t_cmax (thelast only for HeapTupleSelfUpdated, since we
2368+
*cannot obtain cmax from a combocid generated by another transaction).
2369+
*See comments for struct HeapUpdateFailureData for additional info.
23712370
*/
23722371
HTSU_Result
23732372
heap_delete(Relationrelation,ItemPointertid,
2374-
ItemPointerctid,TransactionId*update_xmax,
2375-
CommandIdcid,Snapshotcrosscheck,boolwait)
2373+
CommandIdcid,Snapshotcrosscheck,boolwait,
2374+
HeapUpdateFailureData*hufd)
23762375
{
23772376
HTSU_Resultresult;
23782377
TransactionIdxid=GetCurrentTransactionId();
@@ -2533,8 +2532,12 @@ heap_delete(Relation relation, ItemPointer tid,
25332532
result==HeapTupleUpdated||
25342533
result==HeapTupleBeingUpdated);
25352534
Assert(!(tp.t_data->t_infomask&HEAP_XMAX_INVALID));
2536-
*ctid=tp.t_data->t_ctid;
2537-
*update_xmax=HeapTupleHeaderGetXmax(tp.t_data);
2535+
hufd->ctid=tp.t_data->t_ctid;
2536+
hufd->xmax=HeapTupleHeaderGetXmax(tp.t_data);
2537+
if (result==HeapTupleSelfUpdated)
2538+
hufd->cmax=HeapTupleHeaderGetCmax(tp.t_data);
2539+
else
2540+
hufd->cmax=0;/* for lack of an InvalidCommandId value */
25382541
UnlockReleaseBuffer(buffer);
25392542
if (have_tuple_lock)
25402543
UnlockTuple(relation,&(tp.t_self),ExclusiveLock);
@@ -2666,13 +2669,12 @@ void
26662669
simple_heap_delete(Relationrelation,ItemPointertid)
26672670
{
26682671
HTSU_Resultresult;
2669-
ItemPointerDataupdate_ctid;
2670-
TransactionIdupdate_xmax;
2672+
HeapUpdateFailureDatahufd;
26712673

26722674
result=heap_delete(relation,tid,
2673-
&update_ctid,&update_xmax,
26742675
GetCurrentCommandId(true),InvalidSnapshot,
2675-
true/* wait for commit */ );
2676+
true/* wait for commit */,
2677+
&hufd);
26762678
switch (result)
26772679
{
26782680
caseHeapTupleSelfUpdated:
@@ -2703,12 +2705,11 @@ simple_heap_delete(Relation relation, ItemPointer tid)
27032705
*relation - table to be modified (caller must hold suitable lock)
27042706
*otid - TID of old tuple to be replaced
27052707
*newtup - newly constructed tuple data to store
2706-
*ctid - output parameter, used only for failure case (see below)
2707-
*update_xmax - output parameter, used only for failure case (see below)
27082708
*cid - update command ID (used for visibility test, and stored into
27092709
*cmax/cmin if successful)
27102710
*crosscheck - if not InvalidSnapshot, also check old tuple against this
27112711
*wait - true if should wait for any conflicting update to commit/abort
2712+
*hufd - output parameter, filled in failure cases (see below)
27122713
*
27132714
* Normal, successful return value is HeapTupleMayBeUpdated, which
27142715
* actually means we *did* update it. Failure return codes are
@@ -2721,15 +2722,15 @@ simple_heap_delete(Relation relation, ItemPointer tid)
27212722
* update was done. However, any TOAST changes in the new tuple's
27222723
* data are not reflected into *newtup.
27232724
*
2724-
* In the failure cases, the routinereturnsthe tuple's t_ctid and t_xmax.
2725-
*If t_ctid isthesame as otid, the tuple was deleted; if different, the
2726-
*tuple was updated, and t_ctid is the location of the replacement tuple.
2727-
*(t_xmax is needed to verify that the replacement tuple matches.)
2725+
* In the failure cases, the routinefills *hufd withthe tuple's t_ctid,
2726+
*t_xmax, and t_cmax (thelast only for HeapTupleSelfUpdated, since we
2727+
*cannot obtain cmax from a combocid generated by another transaction).
2728+
*See comments for struct HeapUpdateFailureData for additional info.
27282729
*/
27292730
HTSU_Result
27302731
heap_update(Relationrelation,ItemPointerotid,HeapTuplenewtup,
2731-
ItemPointerctid,TransactionId*update_xmax,
2732-
CommandIdcid,Snapshotcrosscheck,boolwait)
2732+
CommandIdcid,Snapshotcrosscheck,boolwait,
2733+
HeapUpdateFailureData*hufd)
27332734
{
27342735
HTSU_Resultresult;
27352736
TransactionIdxid=GetCurrentTransactionId();
@@ -2908,8 +2909,12 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
29082909
result==HeapTupleUpdated||
29092910
result==HeapTupleBeingUpdated);
29102911
Assert(!(oldtup.t_data->t_infomask&HEAP_XMAX_INVALID));
2911-
*ctid=oldtup.t_data->t_ctid;
2912-
*update_xmax=HeapTupleHeaderGetXmax(oldtup.t_data);
2912+
hufd->ctid=oldtup.t_data->t_ctid;
2913+
hufd->xmax=HeapTupleHeaderGetXmax(oldtup.t_data);
2914+
if (result==HeapTupleSelfUpdated)
2915+
hufd->cmax=HeapTupleHeaderGetCmax(oldtup.t_data);
2916+
else
2917+
hufd->cmax=0;/* for lack of an InvalidCommandId value */
29132918
UnlockReleaseBuffer(buffer);
29142919
if (have_tuple_lock)
29152920
UnlockTuple(relation,&(oldtup.t_self),ExclusiveLock);
@@ -3379,13 +3384,12 @@ void
33793384
simple_heap_update(Relationrelation,ItemPointerotid,HeapTupletup)
33803385
{
33813386
HTSU_Resultresult;
3382-
ItemPointerDataupdate_ctid;
3383-
TransactionIdupdate_xmax;
3387+
HeapUpdateFailureDatahufd;
33843388

33853389
result=heap_update(relation,otid,tup,
3386-
&update_ctid,&update_xmax,
33873390
GetCurrentCommandId(true),InvalidSnapshot,
3388-
true/* wait for commit */ );
3391+
true/* wait for commit */,
3392+
&hufd);
33893393
switch (result)
33903394
{
33913395
caseHeapTupleSelfUpdated:
@@ -3423,18 +3427,17 @@ simple_heap_update(Relation relation, ItemPointer otid, HeapTuple tup)
34233427
* Output parameters:
34243428
**tuple: all fields filled in
34253429
**buffer: set to buffer holding tuple (pinned but not locked at exit)
3426-
**ctid: set to tuple's t_ctid, but only in failure cases
3427-
**update_xmax: set to tuple's xmax, but only in failure cases
3430+
**hufd: filled in failure cases (see below)
34283431
*
34293432
* Function result may be:
34303433
*HeapTupleMayBeUpdated: lock was successfully acquired
34313434
*HeapTupleSelfUpdated: lock failed because tuple updated by self
34323435
*HeapTupleUpdated: lock failed because tuple updated by other xact
34333436
*
3434-
* In the failure cases, the routinereturnsthe tuple's t_ctid and t_xmax.
3435-
*If t_ctid isthesame as t_self, the tuple was deleted; if different, the
3436-
*tuple was updated, and t_ctid is the location of the replacement tuple.
3437-
*(t_xmax is needed to verify that the replacement tuple matches.)
3437+
* In the failure cases, the routinefills *hufd withthe tuple's t_ctid,
3438+
*t_xmax, and t_cmax (thelast only for HeapTupleSelfUpdated, since we
3439+
*cannot obtain cmax from a combocid generated by another transaction).
3440+
*See comments for struct HeapUpdateFailureData for additional info.
34383441
*
34393442
*
34403443
* NOTES: because the shared-memory lock table is of finite size, but users
@@ -3470,9 +3473,9 @@ simple_heap_update(Relation relation, ItemPointer otid, HeapTuple tup)
34703473
* conflict for a tuple, we don't incur any extra overhead.
34713474
*/
34723475
HTSU_Result
3473-
heap_lock_tuple(Relationrelation,HeapTupletuple,Buffer*buffer,
3474-
ItemPointerctid,TransactionId*update_xmax,
3475-
CommandIdcid,LockTupleModemode,boolnowait)
3476+
heap_lock_tuple(Relationrelation,HeapTupletuple,
3477+
CommandIdcid,LockTupleModemode,boolnowait,
3478+
Buffer*buffer,HeapUpdateFailureData*hufd)
34763479
{
34773480
HTSU_Resultresult;
34783481
ItemPointertid=&(tuple->t_self);
@@ -3657,8 +3660,12 @@ heap_lock_tuple(Relation relation, HeapTuple tuple, Buffer *buffer,
36573660
{
36583661
Assert(result==HeapTupleSelfUpdated||result==HeapTupleUpdated);
36593662
Assert(!(tuple->t_data->t_infomask&HEAP_XMAX_INVALID));
3660-
*ctid=tuple->t_data->t_ctid;
3661-
*update_xmax=HeapTupleHeaderGetXmax(tuple->t_data);
3663+
hufd->ctid=tuple->t_data->t_ctid;
3664+
hufd->xmax=HeapTupleHeaderGetXmax(tuple->t_data);
3665+
if (result==HeapTupleSelfUpdated)
3666+
hufd->cmax=HeapTupleHeaderGetCmax(tuple->t_data);
3667+
else
3668+
hufd->cmax=0;/* for lack of an InvalidCommandId value */
36623669
LockBuffer(*buffer,BUFFER_LOCK_UNLOCK);
36633670
if (have_tuple_lock)
36643671
UnlockTuple(relation,tid,tuple_lock_type);

‎src/backend/commands/trigger.c

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2571,8 +2571,7 @@ GetTupleForTrigger(EState *estate,
25712571
if (newSlot!=NULL)
25722572
{
25732573
HTSU_Resulttest;
2574-
ItemPointerDataupdate_ctid;
2575-
TransactionIdupdate_xmax;
2574+
HeapUpdateFailureDatahufd;
25762575

25772576
*newSlot=NULL;
25782577

@@ -2584,13 +2583,27 @@ GetTupleForTrigger(EState *estate,
25842583
*/
25852584
ltrmark:;
25862585
tuple.t_self=*tid;
2587-
test=heap_lock_tuple(relation,&tuple,&buffer,
2588-
&update_ctid,&update_xmax,
2586+
test=heap_lock_tuple(relation,&tuple,
25892587
estate->es_output_cid,
2590-
LockTupleExclusive, false);
2588+
LockTupleExclusive, false/* wait */,
2589+
&buffer,&hufd);
25912590
switch (test)
25922591
{
25932592
caseHeapTupleSelfUpdated:
2593+
/*
2594+
* The target tuple was already updated or deleted by the
2595+
* current command, or by a later command in the current
2596+
* transaction. We ignore the tuple in the former case, and
2597+
* throw error in the latter case, for the same reasons
2598+
* enumerated in ExecUpdate and ExecDelete in
2599+
* nodeModifyTable.c.
2600+
*/
2601+
if (hufd.cmax!=estate->es_output_cid)
2602+
ereport(ERROR,
2603+
(errcode(ERRCODE_TRIGGERED_DATA_CHANGE_VIOLATION),
2604+
errmsg("tuple to be updated was already modified by an operation triggered by the current command"),
2605+
errhint("Consider using an AFTER trigger instead of a BEFORE trigger to propagate changes to other rows.")));
2606+
25942607
/* treat it as deleted; do not process */
25952608
ReleaseBuffer(buffer);
25962609
returnNULL;
@@ -2604,7 +2617,7 @@ ltrmark:;
26042617
ereport(ERROR,
26052618
(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
26062619
errmsg("could not serialize access due to concurrent update")));
2607-
if (!ItemPointerEquals(&update_ctid,&tuple.t_self))
2620+
if (!ItemPointerEquals(&hufd.ctid,&tuple.t_self))
26082621
{
26092622
/* it was updated, so look at the updated version */
26102623
TupleTableSlot*epqslot;
@@ -2613,11 +2626,11 @@ ltrmark:;
26132626
epqstate,
26142627
relation,
26152628
relinfo->ri_RangeTableIndex,
2616-
&update_ctid,
2617-
update_xmax);
2629+
&hufd.ctid,
2630+
hufd.xmax);
26182631
if (!TupIsNull(epqslot))
26192632
{
2620-
*tid=update_ctid;
2633+
*tid=hufd.ctid;
26212634
*newSlot=epqslot;
26222635

26232636
/*

‎src/backend/executor/execMain.c

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1802,8 +1802,7 @@ EvalPlanQualFetch(EState *estate, Relation relation, int lockmode,
18021802
if (heap_fetch(relation,&SnapshotDirty,&tuple,&buffer, true,NULL))
18031803
{
18041804
HTSU_Resulttest;
1805-
ItemPointerDataupdate_ctid;
1806-
TransactionIdupdate_xmax;
1805+
HeapUpdateFailureDatahufd;
18071806

18081807
/*
18091808
* If xmin isn't what we're expecting, the slot must have been
@@ -1838,13 +1837,13 @@ EvalPlanQualFetch(EState *estate, Relation relation, int lockmode,
18381837
/*
18391838
* If tuple was inserted by our own transaction, we have to check
18401839
* cmin against es_output_cid: cmin >= current CID means our
1841-
* command cannot see the tuple, so we should ignore it. Without
1842-
*this we are open to the "Halloween problem" of indefinitely
1843-
*re-updating the sametuple. (We need not check cmax because
1844-
* HeapTupleSatisfiesDirty will consider a tuple deleted by our
1845-
* transaction dead, regardless of cmax.) We just checked that
1846-
* priorXmax == xmin, so we can test that variable instead of
1847-
* doing HeapTupleHeaderGetXmin again.
1840+
* command cannot see the tuple, so we should ignore it.
1841+
*Otherwise heap_lock_tuple() will throw an error, and so would
1842+
*any later attempt to update or delete thetuple.(We need not
1843+
*check cmax becauseHeapTupleSatisfiesDirty will consider a
1844+
*tuple deleted by ourtransaction dead, regardless of cmax.)
1845+
*Wee just checked thatpriorXmax == xmin, so we can test that
1846+
*variable instead ofdoing HeapTupleHeaderGetXmin again.
18481847
*/
18491848
if (TransactionIdIsCurrentTransactionId(priorXmax)&&
18501849
HeapTupleHeaderGetCmin(tuple.t_data) >=estate->es_output_cid)
@@ -1856,17 +1855,29 @@ EvalPlanQualFetch(EState *estate, Relation relation, int lockmode,
18561855
/*
18571856
* This is a live tuple, so now try to lock it.
18581857
*/
1859-
test=heap_lock_tuple(relation,&tuple,&buffer,
1860-
&update_ctid,&update_xmax,
1858+
test=heap_lock_tuple(relation,&tuple,
18611859
estate->es_output_cid,
1862-
lockmode, false);
1860+
lockmode, false/* wait */,
1861+
&buffer,&hufd);
18631862
/* We now have two pins on the buffer, get rid of one */
18641863
ReleaseBuffer(buffer);
18651864

18661865
switch (test)
18671866
{
18681867
caseHeapTupleSelfUpdated:
1869-
/* treat it as deleted; do not process */
1868+
/*
1869+
* The target tuple was already updated or deleted by the
1870+
* current command, or by a later command in the current
1871+
* transaction. We *must* ignore the tuple in the former
1872+
* case, so as to avoid the "Halloween problem" of
1873+
* repeated update attempts. In the latter case it might
1874+
* be sensible to fetch the updated tuple instead, but
1875+
* doing so would require changing heap_lock_tuple as well
1876+
* as heap_update and heap_delete to not complain about
1877+
* updating "invisible" tuples, which seems pretty scary.
1878+
* So for now, treat the tuple as deleted and do not
1879+
* process.
1880+
*/
18701881
ReleaseBuffer(buffer);
18711882
returnNULL;
18721883

@@ -1880,12 +1891,12 @@ EvalPlanQualFetch(EState *estate, Relation relation, int lockmode,
18801891
ereport(ERROR,
18811892
(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
18821893
errmsg("could not serialize access due to concurrent update")));
1883-
if (!ItemPointerEquals(&update_ctid,&tuple.t_self))
1894+
if (!ItemPointerEquals(&hufd.ctid,&tuple.t_self))
18841895
{
18851896
/* it was updated, so look at the updated version */
1886-
tuple.t_self=update_ctid;
1897+
tuple.t_self=hufd.ctid;
18871898
/* updated row should have xmin matching this xmax */
1888-
priorXmax=update_xmax;
1899+
priorXmax=hufd.xmax;
18891900
continue;
18901901
}
18911902
/* tuple was deleted, so give up */

‎src/backend/executor/nodeLockRows.c

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,7 @@ ExecLockRows(LockRowsState *node)
7171
boolisNull;
7272
HeapTupleDatatuple;
7373
Bufferbuffer;
74-
ItemPointerDataupdate_ctid;
75-
TransactionIdupdate_xmax;
74+
HeapUpdateFailureDatahufd;
7675
LockTupleModelockmode;
7776
HTSU_Resulttest;
7877
HeapTuplecopyTuple;
@@ -117,15 +116,26 @@ ExecLockRows(LockRowsState *node)
117116
else
118117
lockmode=LockTupleShared;
119118

120-
test=heap_lock_tuple(erm->relation,&tuple,&buffer,
121-
&update_ctid,&update_xmax,
119+
test=heap_lock_tuple(erm->relation,&tuple,
122120
estate->es_output_cid,
123-
lockmode,erm->noWait);
121+
lockmode,erm->noWait,
122+
&buffer,&hufd);
124123
ReleaseBuffer(buffer);
125124
switch (test)
126125
{
127126
caseHeapTupleSelfUpdated:
128-
/* treat it as deleted; do not process */
127+
/*
128+
* The target tuple was already updated or deleted by the
129+
* current command, or by a later command in the current
130+
* transaction. We *must* ignore the tuple in the former
131+
* case, so as to avoid the "Halloween problem" of repeated
132+
* update attempts. In the latter case it might be sensible
133+
* to fetch the updated tuple instead, but doing so would
134+
* require changing heap_lock_tuple as well as heap_update and
135+
* heap_delete to not complain about updating "invisible"
136+
* tuples, which seems pretty scary. So for now, treat the
137+
* tuple as deleted and do not process.
138+
*/
129139
gotolnext;
130140

131141
caseHeapTupleMayBeUpdated:
@@ -137,16 +147,15 @@ ExecLockRows(LockRowsState *node)
137147
ereport(ERROR,
138148
(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
139149
errmsg("could not serialize access due to concurrent update")));
140-
if (ItemPointerEquals(&update_ctid,
141-
&tuple.t_self))
150+
if (ItemPointerEquals(&hufd.ctid,&tuple.t_self))
142151
{
143152
/* Tuple was deleted, so don't return it */
144153
gotolnext;
145154
}
146155

147156
/* updated, so fetch and lock the updated version */
148157
copyTuple=EvalPlanQualFetch(estate,erm->relation,lockmode,
149-
&update_ctid,update_xmax);
158+
&hufd.ctid,hufd.xmax);
150159

151160
if (copyTuple==NULL)
152161
{

0 commit comments

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