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
Appearance settings

Commit 193e6d1

Browse filesBrowse files
akorotkovakorotkov
committed
Revert: Allow locking updated tuples in tuple_update() and tuple_delete()
This commit reverts 87985cc and 818861e per review by Andres Freund. Discussion: https://postgr.es/m/20240410165236.rwyrny7ihi4ddxw4%40awork3.anarazel.de
1 parent da841aa commit 193e6d1
Copy full SHA for 193e6d1

File tree

Expand file treeCollapse file tree

9 files changed

+346
-501
lines changed
Filter options
Expand file treeCollapse file tree

9 files changed

+346
-501
lines changed

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

Copy file name to clipboardExpand all lines: src/backend/access/heap/heapam.c
+50-155Lines changed: 50 additions & 155 deletions
Large diffs are not rendered by default.

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

Copy file name to clipboardExpand all lines: src/backend/access/heap/heapam_handler.c
+18-75Lines changed: 18 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,6 @@
4646
#include "utils/builtins.h"
4747
#include "utils/rel.h"
4848

49-
static TM_Result heapam_tuple_lock(Relation relation, ItemPointer tid,
50-
Snapshot snapshot, TupleTableSlot *slot,
51-
CommandId cid, LockTupleMode mode,
52-
LockWaitPolicy wait_policy, uint8 flags,
53-
TM_FailureData *tmfd);
5449
static void reform_and_rewrite_tuple(HeapTuple tuple,
5550
Relation OldHeap, Relation NewHeap,
5651
Datum *values, bool *isnull, RewriteState rwstate);
@@ -306,55 +301,23 @@ heapam_tuple_complete_speculative(Relation relation, TupleTableSlot *slot,
306301

307302
static TM_Result
308303
heapam_tuple_delete(Relation relation, ItemPointer tid, CommandId cid,
309-
Snapshot snapshot, Snapshot crosscheck, int options,
310-
TM_FailureData *tmfd, bool changingPart,
311-
TupleTableSlot *oldSlot)
304+
Snapshot snapshot, Snapshot crosscheck, bool wait,
305+
TM_FailureData *tmfd, bool changingPart)
312306
{
313-
TM_Result result;
314-
315307
/*
316308
* Currently Deleting of index tuples are handled at vacuum, in case if
317309
* the storage itself is cleaning the dead tuples by itself, it is the
318310
* time to call the index tuple deletion also.
319311
*/
320-
result = heap_delete(relation, tid, cid, crosscheck, options,
321-
tmfd, changingPart, oldSlot);
322-
323-
/*
324-
* If the tuple has been concurrently updated, then get the lock on it.
325-
* (Do only if caller asked for this by setting the
326-
* TABLE_MODIFY_LOCK_UPDATED option) With the lock held retry of the
327-
* delete should succeed even if there are more concurrent update
328-
* attempts.
329-
*/
330-
if (result == TM_Updated && (options & TABLE_MODIFY_LOCK_UPDATED))
331-
{
332-
/*
333-
* heapam_tuple_lock() will take advantage of tuple loaded into
334-
* oldSlot by heap_delete().
335-
*/
336-
result = heapam_tuple_lock(relation, tid, snapshot,
337-
oldSlot, cid, LockTupleExclusive,
338-
(options & TABLE_MODIFY_WAIT) ?
339-
LockWaitBlock :
340-
LockWaitSkip,
341-
TUPLE_LOCK_FLAG_FIND_LAST_VERSION,
342-
tmfd);
343-
344-
if (result == TM_Ok)
345-
return TM_Updated;
346-
}
347-
348-
return result;
312+
return heap_delete(relation, tid, cid, crosscheck, wait, tmfd, changingPart);
349313
}
350314

351315

352316
static TM_Result
353317
heapam_tuple_update(Relation relation, ItemPointer otid, TupleTableSlot *slot,
354318
CommandId cid, Snapshot snapshot, Snapshot crosscheck,
355-
int options, TM_FailureData *tmfd,
356-
LockTupleMode *lockmode, TU_UpdateIndexes *update_indexes,
357-
TupleTableSlot *oldSlot)
319+
bool wait, TM_FailureData *tmfd,
320+
LockTupleMode *lockmode, TU_UpdateIndexes *update_indexes)
358321
{
359322
bool shouldFree = true;
360323
HeapTuple tuple = ExecFetchSlotHeapTuple(slot, true, &shouldFree);
@@ -364,8 +327,8 @@ heapam_tuple_update(Relation relation, ItemPointer otid, TupleTableSlot *slot,
364327
slot->tts_tableOid = RelationGetRelid(relation);
365328
tuple->t_tableOid = slot->tts_tableOid;
366329

367-
result = heap_update(relation, otid, tuple, cid, crosscheck, options,
368-
tmfd, lockmode, update_indexes, oldSlot);
330+
result = heap_update(relation, otid, tuple, cid, crosscheck, wait,
331+
tmfd, lockmode, update_indexes);
369332
ItemPointerCopy(&tuple->t_self, &slot->tts_tid);
370333

371334
/*
@@ -392,31 +355,6 @@ heapam_tuple_update(Relation relation, ItemPointer otid, TupleTableSlot *slot,
392355
if (shouldFree)
393356
pfree(tuple);
394357

395-
/*
396-
* If the tuple has been concurrently updated, then get the lock on it.
397-
* (Do only if caller asked for this by setting the
398-
* TABLE_MODIFY_LOCK_UPDATED option) With the lock held retry of the
399-
* update should succeed even if there are more concurrent update
400-
* attempts.
401-
*/
402-
if (result == TM_Updated && (options & TABLE_MODIFY_LOCK_UPDATED))
403-
{
404-
/*
405-
* heapam_tuple_lock() will take advantage of tuple loaded into
406-
* oldSlot by heap_update().
407-
*/
408-
result = heapam_tuple_lock(relation, otid, snapshot,
409-
oldSlot, cid, *lockmode,
410-
(options & TABLE_MODIFY_WAIT) ?
411-
LockWaitBlock :
412-
LockWaitSkip,
413-
TUPLE_LOCK_FLAG_FIND_LAST_VERSION,
414-
tmfd);
415-
416-
if (result == TM_Ok)
417-
return TM_Updated;
418-
}
419-
420358
return result;
421359
}
422360

@@ -428,6 +366,7 @@ heapam_tuple_lock(Relation relation, ItemPointer tid, Snapshot snapshot,
428366
{
429367
BufferHeapTupleTableSlot *bslot = (BufferHeapTupleTableSlot *) slot;
430368
TM_Result result;
369+
Buffer buffer;
431370
HeapTuple tuple = &bslot->base.tupdata;
432371
bool follow_updates;
433372

@@ -437,15 +376,18 @@ heapam_tuple_lock(Relation relation, ItemPointer tid, Snapshot snapshot,
437376
Assert(TTS_IS_BUFFERTUPLE(slot));
438377

439378
tuple_lock_retry:
440-
result = heap_lock_tuple(relation, tid, slot, cid, mode, wait_policy,
441-
follow_updates, tmfd);
379+
tuple->t_self = *tid;
380+
result = heap_lock_tuple(relation, tuple, cid, mode, wait_policy,
381+
follow_updates, &buffer, tmfd);
442382

443383
if (result == TM_Updated &&
444384
(flags & TUPLE_LOCK_FLAG_FIND_LAST_VERSION))
445385
{
446386
/* Should not encounter speculative tuple on recheck */
447387
Assert(!HeapTupleHeaderIsSpeculative(tuple->t_data));
448388

389+
ReleaseBuffer(buffer);
390+
449391
if (!ItemPointerEquals(&tmfd->ctid, &tuple->t_self))
450392
{
451393
SnapshotData SnapshotDirty;
@@ -467,8 +409,6 @@ heapam_tuple_lock(Relation relation, ItemPointer tid, Snapshot snapshot,
467409
InitDirtySnapshot(SnapshotDirty);
468410
for (;;)
469411
{
470-
Buffer buffer = InvalidBuffer;
471-
472412
if (ItemPointerIndicatesMovedPartitions(tid))
473413
ereport(ERROR,
474414
(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
@@ -563,7 +503,7 @@ heapam_tuple_lock(Relation relation, ItemPointer tid, Snapshot snapshot,
563503
/*
564504
* This is a live tuple, so try to lock it again.
565505
*/
566-
ExecStorePinnedBufferHeapTuple(tuple, slot, buffer);
506+
ReleaseBuffer(buffer);
567507
goto tuple_lock_retry;
568508
}
569509

@@ -574,7 +514,7 @@ heapam_tuple_lock(Relation relation, ItemPointer tid, Snapshot snapshot,
574514
*/
575515
if (tuple->t_data == NULL)
576516
{
577-
ReleaseBuffer(buffer);
517+
Assert(!BufferIsValid(buffer));
578518
return TM_Deleted;
579519
}
580520

@@ -627,6 +567,9 @@ heapam_tuple_lock(Relation relation, ItemPointer tid, Snapshot snapshot,
627567
slot->tts_tableOid = RelationGetRelid(relation);
628568
tuple->t_tableOid = slot->tts_tableOid;
629569

570+
/* store in slot, transferring existing pin */
571+
ExecStorePinnedBufferHeapTuple(tuple, slot, buffer);
572+
630573
return result;
631574
}
632575

‎src/backend/access/table/tableam.c

Copy file name to clipboardExpand all lines: src/backend/access/table/tableam.c
+6-20Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -287,23 +287,16 @@ simple_table_tuple_insert(Relation rel, TupleTableSlot *slot)
287287
* via ereport().
288288
*/
289289
void
290-
simple_table_tuple_delete(Relation rel, ItemPointer tid, Snapshot snapshot,
291-
TupleTableSlot *oldSlot)
290+
simple_table_tuple_delete(Relation rel, ItemPointer tid, Snapshot snapshot)
292291
{
293292
TM_Result result;
294293
TM_FailureData tmfd;
295-
int options = TABLE_MODIFY_WAIT; /* wait for commit */
296-
297-
/* Fetch old tuple if the relevant slot is provided */
298-
if (oldSlot)
299-
options |= TABLE_MODIFY_FETCH_OLD_TUPLE;
300294

301295
result = table_tuple_delete(rel, tid,
302296
GetCurrentCommandId(true),
303297
snapshot, InvalidSnapshot,
304-
options,
305-
&tmfd, false /* changingPart */ ,
306-
oldSlot);
298+
true /* wait for commit */ ,
299+
&tmfd, false /* changingPart */ );
307300

308301
switch (result)
309302
{
@@ -342,24 +335,17 @@ void
342335
simple_table_tuple_update(Relation rel, ItemPointer otid,
343336
TupleTableSlot *slot,
344337
Snapshot snapshot,
345-
TU_UpdateIndexes *update_indexes,
346-
TupleTableSlot *oldSlot)
338+
TU_UpdateIndexes *update_indexes)
347339
{
348340
TM_Result result;
349341
TM_FailureData tmfd;
350342
LockTupleMode lockmode;
351-
int options = TABLE_MODIFY_WAIT; /* wait for commit */
352-
353-
/* Fetch old tuple if the relevant slot is provided */
354-
if (oldSlot)
355-
options |= TABLE_MODIFY_FETCH_OLD_TUPLE;
356343

357344
result = table_tuple_update(rel, otid, slot,
358345
GetCurrentCommandId(true),
359346
snapshot, InvalidSnapshot,
360-
options,
361-
&tmfd, &lockmode, update_indexes,
362-
oldSlot);
347+
true /* wait for commit */ ,
348+
&tmfd, &lockmode, update_indexes);
363349

364350
switch (result)
365351
{

‎src/backend/commands/trigger.c

Copy file name to clipboardExpand all lines: src/backend/commands/trigger.c
+40-15Lines changed: 40 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2773,8 +2773,8 @@ ExecBRDeleteTriggers(EState *estate, EPQState *epqstate,
27732773
void
27742774
ExecARDeleteTriggers(EState *estate,
27752775
ResultRelInfo *relinfo,
2776+
ItemPointer tupleid,
27762777
HeapTuple fdw_trigtuple,
2777-
TupleTableSlot *slot,
27782778
TransitionCaptureState *transition_capture,
27792779
bool is_crosspart_update)
27802780
{
@@ -2783,11 +2783,20 @@ ExecARDeleteTriggers(EState *estate,
27832783
if ((trigdesc && trigdesc->trig_delete_after_row) ||
27842784
(transition_capture && transition_capture->tcs_delete_old_table))
27852785
{
2786-
/*
2787-
* Put the FDW old tuple to the slot. Otherwise, the caller is
2788-
* expected to have an old tuple already fetched to the slot.
2789-
*/
2790-
if (fdw_trigtuple != NULL)
2786+
TupleTableSlot *slot = ExecGetTriggerOldSlot(estate, relinfo);
2787+
2788+
Assert(HeapTupleIsValid(fdw_trigtuple) ^ ItemPointerIsValid(tupleid));
2789+
if (fdw_trigtuple == NULL)
2790+
GetTupleForTrigger(estate,
2791+
NULL,
2792+
relinfo,
2793+
tupleid,
2794+
LockTupleExclusive,
2795+
slot,
2796+
NULL,
2797+
NULL,
2798+
NULL);
2799+
else
27912800
ExecForceStoreHeapTuple(fdw_trigtuple, slot, false);
27922801

27932802
AfterTriggerSaveEvent(estate, relinfo, NULL, NULL,
@@ -3078,17 +3087,18 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
30783087
* Note: 'src_partinfo' and 'dst_partinfo', when non-NULL, refer to the source
30793088
* and destination partitions, respectively, of a cross-partition update of
30803089
* the root partitioned table mentioned in the query, given by 'relinfo'.
3081-
* 'oldslot' contains the "old" tuple in the source partition, and 'newslot'
3082-
* contains the "new" tuple in the destination partition. This interface
3083-
* allows to support the requirements of ExecCrossPartitionUpdateForeignKey();
3084-
* is_crosspart_update must be true in that case.
3090+
* 'tupleid' in that case refers to the ctid of the "old" tuple in the source
3091+
* partition, and 'newslot' contains the "new" tuple in the destination
3092+
* partition. This interface allows to support the requirements of
3093+
* ExecCrossPartitionUpdateForeignKey(); is_crosspart_update must be true in
3094+
* that case.
30853095
*/
30863096
void
30873097
ExecARUpdateTriggers(EState *estate, ResultRelInfo *relinfo,
30883098
ResultRelInfo *src_partinfo,
30893099
ResultRelInfo *dst_partinfo,
3100+
ItemPointer tupleid,
30903101
HeapTuple fdw_trigtuple,
3091-
TupleTableSlot *oldslot,
30923102
TupleTableSlot *newslot,
30933103
List *recheckIndexes,
30943104
TransitionCaptureState *transition_capture,
@@ -3107,14 +3117,29 @@ ExecARUpdateTriggers(EState *estate, ResultRelInfo *relinfo,
31073117
* separately for DELETE and INSERT to capture transition table rows.
31083118
* In such case, either old tuple or new tuple can be NULL.
31093119
*/
3120+
TupleTableSlot *oldslot;
3121+
ResultRelInfo *tupsrc;
3122+
31103123
Assert((src_partinfo != NULL && dst_partinfo != NULL) ||
31113124
!is_crosspart_update);
31123125

3113-
if (fdw_trigtuple != NULL)
3114-
{
3115-
Assert(oldslot);
3126+
tupsrc = src_partinfo ? src_partinfo : relinfo;
3127+
oldslot = ExecGetTriggerOldSlot(estate, tupsrc);
3128+
3129+
if (fdw_trigtuple == NULL && ItemPointerIsValid(tupleid))
3130+
GetTupleForTrigger(estate,
3131+
NULL,
3132+
tupsrc,
3133+
tupleid,
3134+
LockTupleExclusive,
3135+
oldslot,
3136+
NULL,
3137+
NULL,
3138+
NULL);
3139+
else if (fdw_trigtuple != NULL)
31163140
ExecForceStoreHeapTuple(fdw_trigtuple, oldslot, false);
3117-
}
3141+
else
3142+
ExecClearTuple(oldslot);
31183143

31193144
AfterTriggerSaveEvent(estate, relinfo,
31203145
src_partinfo, dst_partinfo,

‎src/backend/executor/execReplication.c

Copy file name to clipboardExpand all lines: src/backend/executor/execReplication.c
+4-15Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -577,7 +577,6 @@ ExecSimpleRelationUpdate(ResultRelInfo *resultRelInfo,
577577
{
578578
List *recheckIndexes = NIL;
579579
TU_UpdateIndexes update_indexes;
580-
TupleTableSlot *oldSlot = NULL;
581580

582581
/* Compute stored generated columns */
583582
if (rel->rd_att->constr &&
@@ -591,12 +590,8 @@ ExecSimpleRelationUpdate(ResultRelInfo *resultRelInfo,
591590
if (rel->rd_rel->relispartition)
592591
ExecPartitionCheck(resultRelInfo, slot, estate, true);
593592

594-
if (resultRelInfo->ri_TrigDesc &&
595-
resultRelInfo->ri_TrigDesc->trig_update_after_row)
596-
oldSlot = ExecGetTriggerOldSlot(estate, resultRelInfo);
597-
598593
simple_table_tuple_update(rel, tid, slot, estate->es_snapshot,
599-
&update_indexes, oldSlot);
594+
&update_indexes);
600595

601596
if (resultRelInfo->ri_NumIndices > 0 && (update_indexes != TU_None))
602597
recheckIndexes = ExecInsertIndexTuples(resultRelInfo,
@@ -607,7 +602,7 @@ ExecSimpleRelationUpdate(ResultRelInfo *resultRelInfo,
607602
/* AFTER ROW UPDATE Triggers */
608603
ExecARUpdateTriggers(estate, resultRelInfo,
609604
NULL, NULL,
610-
NULL, oldSlot, slot,
605+
tid, NULL, slot,
611606
recheckIndexes, NULL, false);
612607

613608
list_free(recheckIndexes);
@@ -641,18 +636,12 @@ ExecSimpleRelationDelete(ResultRelInfo *resultRelInfo,
641636

642637
if (!skip_tuple)
643638
{
644-
TupleTableSlot *oldSlot = NULL;
645-
646-
if (resultRelInfo->ri_TrigDesc &&
647-
resultRelInfo->ri_TrigDesc->trig_delete_after_row)
648-
oldSlot = ExecGetTriggerOldSlot(estate, resultRelInfo);
649-
650639
/* OK, delete the tuple */
651-
simple_table_tuple_delete(rel, tid, estate->es_snapshot, oldSlot);
640+
simple_table_tuple_delete(rel, tid, estate->es_snapshot);
652641

653642
/* AFTER ROW DELETE Triggers */
654643
ExecARDeleteTriggers(estate, resultRelInfo,
655-
NULL, oldSlot, NULL, false);
644+
tid, NULL, NULL, false);
656645
}
657646
}
658647

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.