Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 9321c79

Browse files
committed
Fix concurrent update issues with MERGE.
If MERGE attempts an UPDATE or DELETE on a table with BEFORE ROW triggers, or a cross-partition UPDATE (with or without triggers), and a concurrent UPDATE or DELETE happens, the merge code would fail. In some cases this would lead to a crash, while in others it would cause the wrong merge action to be executed, or no action at all. The immediate cause of the crash was the trigger code calling ExecGetUpdateNewTuple() as part of the EPQ mechanism, which fails because during a merge ri_projectNew is NULL, since merge has its own per-action projection information, which ExecGetUpdateNewTuple() knows nothing about. Fix by arranging for the trigger code to exit early, returning the TM_Result and TM_FailureData information, if a concurrent modification is detected, allowing the merge code to do the necessary EPQ handling in its own way. Similarly, prevent the cross-partition update code from doing any EPQ processing for a merge, allowing the merge code to work out what it needs to do. This leads to a number of simplifications in nodeModifyTable.c. Most notably, the ModifyTableContext->GetUpdateNewTuple() callback is no longer needed, and mergeGetUpdateNewTuple() can be deleted, since there is no longer any requirement for get-update-new-tuple during a merge. Similarly, ModifyTableContext->cpUpdateRetrySlot is no longer needed. Thus ExecGetUpdateNewTuple() and the retry_slot handling of ExecCrossPartitionUpdate() can be restored to how they were in v14, before the merge code was added, and ExecMergeMatched() no longer needs any special-case handling for cross-partition updates. While at it, tidy up ExecUpdateEpilogue() a bit, making it handle recheckIndexes locally, rather than passing it in as a parameter, ensuring that it is freed properly. This dates back to when it was split off from ExecUpdate() to support merge. Per bug #17809 from Alexander Lakhin, and follow-up investigation of bug #17792, also from Alexander Lakhin. Back-patch to v15, where MERGE was introduced, taking care to preserve backwards-compatibility of the trigger API in v15 for any extensions that might use it. Discussion: https://postgr.es/m/17809-9e6650bef133f0fe%40postgresql.org https://postgr.es/m/17792-0f89452029662c36%40postgresql.org
1 parent b2bd9a6 commit 9321c79

File tree

8 files changed

+636
-174
lines changed

8 files changed

+636
-174
lines changed

src/backend/commands/trigger.c

+24-3
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ static bool GetTupleForTrigger(EState *estate,
8787
LockTupleMode lockmode,
8888
TupleTableSlot *oldslot,
8989
TupleTableSlot **epqslot,
90+
TM_Result *tmresultp,
9091
TM_FailureData *tmfdp);
9192
static bool TriggerEnabled(EState *estate, ResultRelInfo *relinfo,
9293
Trigger *trigger, TriggerEvent event,
@@ -2694,7 +2695,9 @@ ExecBRDeleteTriggers(EState *estate, EPQState *epqstate,
26942695
ResultRelInfo *relinfo,
26952696
ItemPointer tupleid,
26962697
HeapTuple fdw_trigtuple,
2697-
TupleTableSlot **epqslot)
2698+
TupleTableSlot **epqslot,
2699+
TM_Result *tmresult,
2700+
TM_FailureData *tmfd)
26982701
{
26992702
TupleTableSlot *slot = ExecGetTriggerOldSlot(estate, relinfo);
27002703
TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
@@ -2711,7 +2714,7 @@ ExecBRDeleteTriggers(EState *estate, EPQState *epqstate,
27112714

27122715
if (!GetTupleForTrigger(estate, epqstate, relinfo, tupleid,
27132716
LockTupleExclusive, slot, &epqslot_candidate,
2714-
NULL))
2717+
tmresult, tmfd))
27152718
return false;
27162719

27172720
/*
@@ -2802,6 +2805,7 @@ ExecARDeleteTriggers(EState *estate,
28022805
LockTupleExclusive,
28032806
slot,
28042807
NULL,
2808+
NULL,
28052809
NULL);
28062810
else
28072811
ExecForceStoreHeapTuple(fdw_trigtuple, slot, false);
@@ -2943,6 +2947,7 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
29432947
ItemPointer tupleid,
29442948
HeapTuple fdw_trigtuple,
29452949
TupleTableSlot *newslot,
2950+
TM_Result *tmresult,
29462951
TM_FailureData *tmfd)
29472952
{
29482953
TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
@@ -2967,7 +2972,7 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
29672972
/* get a copy of the on-disk tuple we are planning to update */
29682973
if (!GetTupleForTrigger(estate, epqstate, relinfo, tupleid,
29692974
lockmode, oldslot, &epqslot_candidate,
2970-
tmfd))
2975+
tmresult, tmfd))
29712976
return false; /* cancel the update action */
29722977

29732978
/*
@@ -3122,6 +3127,7 @@ ExecARUpdateTriggers(EState *estate, ResultRelInfo *relinfo,
31223127
LockTupleExclusive,
31233128
oldslot,
31243129
NULL,
3130+
NULL,
31253131
NULL);
31263132
else if (fdw_trigtuple != NULL)
31273133
ExecForceStoreHeapTuple(fdw_trigtuple, oldslot, false);
@@ -3277,6 +3283,7 @@ GetTupleForTrigger(EState *estate,
32773283
LockTupleMode lockmode,
32783284
TupleTableSlot *oldslot,
32793285
TupleTableSlot **epqslot,
3286+
TM_Result *tmresultp,
32803287
TM_FailureData *tmfdp)
32813288
{
32823289
Relation relation = relinfo->ri_RelationDesc;
@@ -3304,6 +3311,8 @@ GetTupleForTrigger(EState *estate,
33043311
&tmfd);
33053312

33063313
/* Let the caller know about the status of this operation */
3314+
if (tmresultp)
3315+
*tmresultp = test;
33073316
if (tmfdp)
33083317
*tmfdp = tmfd;
33093318

@@ -3331,6 +3340,18 @@ GetTupleForTrigger(EState *estate,
33313340
case TM_Ok:
33323341
if (tmfd.traversed)
33333342
{
3343+
/*
3344+
* Recheck the tuple using EPQ. For MERGE, we leave this
3345+
* to the caller (it must do additional rechecking, and
3346+
* might end up executing a different action entirely).
3347+
*/
3348+
if (estate->es_plannedstmt->commandType == CMD_MERGE)
3349+
{
3350+
if (tmresultp)
3351+
*tmresultp = TM_Updated;
3352+
return false;
3353+
}
3354+
33343355
*epqslot = EvalPlanQual(epqstate,
33353356
relation,
33363357
relinfo->ri_RangeTableIndex,

src/backend/executor/execReplication.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -486,7 +486,7 @@ ExecSimpleRelationUpdate(ResultRelInfo *resultRelInfo,
486486
resultRelInfo->ri_TrigDesc->trig_update_before_row)
487487
{
488488
if (!ExecBRUpdateTriggers(estate, epqstate, resultRelInfo,
489-
tid, NULL, slot, NULL))
489+
tid, NULL, slot, NULL, NULL))
490490
skip_tuple = true; /* "do nothing" */
491491
}
492492

@@ -547,7 +547,7 @@ ExecSimpleRelationDelete(ResultRelInfo *resultRelInfo,
547547
resultRelInfo->ri_TrigDesc->trig_delete_before_row)
548548
{
549549
skip_tuple = !ExecBRDeleteTriggers(estate, epqstate, resultRelInfo,
550-
tid, NULL, NULL);
550+
tid, NULL, NULL, NULL, NULL);
551551
}
552552

553553
if (!skip_tuple)

0 commit comments

Comments
 (0)