Skip to content

Commit 27c7c11

Browse files
deanrasheeddeanrasheed
committed
Fix concurrent update trigger issues with MERGE in a CTE.
If a MERGE inside a CTE attempts an UPDATE or DELETE on a table with BEFORE ROW triggers, and a concurrent UPDATE or DELETE happens, the merge code would fail (crashing in the case of an UPDATE action, and potentially executing the wrong action for a DELETE action). This is the same issue that 9321c79 attempted to fix, except now for a MERGE inside a CTE. As noted in 9321c79, what needs to happen is for the trigger code to exit early, returning the TM_Result and TM_FailureData information to the merge code, if a concurrent modification is detected, rather than attempting to do an EPQ recheck. The merge code will then do its own rechecking, and rescan the action list, potentially executing a different action in light of the concurrent update. In particular, the trigger code must never call ExecGetUpdateNewTuple() for MERGE, since that is bound to fail because MERGE has its own per-action projection information. Commit 9321c79 did this using estate->es_plannedstmt->commandType in the trigger code to detect that a MERGE was being executed, which is fine for a plain MERGE command, but does not work for a MERGE inside a CTE. Fix by passing that information to the trigger code as an additional parameter passed to ExecBRUpdateTriggers() and ExecBRDeleteTriggers(). Back-patch as far as v17 only, since MERGE cannot appear inside a CTE prior to that. Additionally, take care to preserve the trigger ABI in v17 (though not in v18, which is still in beta). Bug: #18986 Reported-by: Yaroslav Syrytsia <me@ys.lc> Author: Dean Rasheed <dean.a.rasheed@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/18986-e7a8aac3d339fa47@postgresql.org Backpatch-through: 17
1 parent bfa9b25 commit 27c7c11

File tree

6 files changed

+89
-50
lines changed

6 files changed

+89
-50
lines changed

‎src/backend/commands/trigger.c

Lines changed: 48 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ static bool GetTupleForTrigger(EState *estate,
8080
ItemPointer tid,
8181
LockTupleMode lockmode,
8282
TupleTableSlot *oldslot,
83+
bool do_epq_recheck,
8384
TupleTableSlot **epqslot,
8485
TM_Result *tmresultp,
8586
TM_FailureData *tmfdp);
@@ -2693,7 +2694,8 @@ ExecBRDeleteTriggers(EState *estate, EPQState *epqstate,
26932694
HeapTuple fdw_trigtuple,
26942695
TupleTableSlot **epqslot,
26952696
TM_Result *tmresult,
2696-
TM_FailureData *tmfd)
2697+
TM_FailureData *tmfd,
2698+
bool is_merge_delete)
26972699
{
26982700
TupleTableSlot *slot = ExecGetTriggerOldSlot(estate, relinfo);
26992701
TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
@@ -2708,9 +2710,17 @@ ExecBRDeleteTriggers(EState *estate, EPQState *epqstate,
27082710
{
27092711
TupleTableSlot *epqslot_candidate = NULL;
27102712

2713+
/*
2714+
* Get a copy of the on-disk tuple we are planning to delete. In
2715+
* general, if the tuple has been concurrently updated, we should
2716+
* recheck it using EPQ. However, if this is a MERGE DELETE action,
2717+
* we skip this EPQ recheck and leave it to the caller (it must do
2718+
* additional rechecking, and might end up executing a different
2719+
* action entirely).
2720+
*/
27112721
if (!GetTupleForTrigger(estate, epqstate, relinfo, tupleid,
2712-
LockTupleExclusive, slot, &epqslot_candidate,
2713-
tmresult, tmfd))
2722+
LockTupleExclusive, slot, !is_merge_delete,
2723+
&epqslot_candidate, tmresult, tmfd))
27142724
return false;
27152725

27162726
/*
@@ -2800,6 +2810,7 @@ ExecARDeleteTriggers(EState *estate,
28002810
tupleid,
28012811
LockTupleExclusive,
28022812
slot,
2813+
false,
28032814
NULL,
28042815
NULL,
28052816
NULL);
@@ -2944,7 +2955,8 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
29442955
HeapTuple fdw_trigtuple,
29452956
TupleTableSlot *newslot,
29462957
TM_Result *tmresult,
2947-
TM_FailureData *tmfd)
2958+
TM_FailureData *tmfd,
2959+
bool is_merge_update)
29482960
{
29492961
TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
29502962
TupleTableSlot *oldslot = ExecGetTriggerOldSlot(estate, relinfo);
@@ -2965,10 +2977,17 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
29652977
{
29662978
TupleTableSlot *epqslot_candidate = NULL;
29672979

2968-
/* get a copy of the on-disk tuple we are planning to update */
2980+
/*
2981+
* Get a copy of the on-disk tuple we are planning to update. In
2982+
* general, if the tuple has been concurrently updated, we should
2983+
* recheck it using EPQ. However, if this is a MERGE UPDATE action,
2984+
* we skip this EPQ recheck and leave it to the caller (it must do
2985+
* additional rechecking, and might end up executing a different
2986+
* action entirely).
2987+
*/
29692988
if (!GetTupleForTrigger(estate, epqstate, relinfo, tupleid,
2970-
lockmode, oldslot, &epqslot_candidate,
2971-
tmresult, tmfd))
2989+
lockmode, oldslot, !is_merge_update,
2990+
&epqslot_candidate, tmresult, tmfd))
29722991
return false; /* cancel the update action */
29732992

29742993
/*
@@ -3142,6 +3161,7 @@ ExecARUpdateTriggers(EState *estate, ResultRelInfo *relinfo,
31423161
tupleid,
31433162
LockTupleExclusive,
31443163
oldslot,
3164+
false,
31453165
NULL,
31463166
NULL,
31473167
NULL);
@@ -3298,6 +3318,7 @@ GetTupleForTrigger(EState *estate,
32983318
ItemPointer tid,
32993319
LockTupleMode lockmode,
33003320
TupleTableSlot *oldslot,
3321+
bool do_epq_recheck,
33013322
TupleTableSlot **epqslot,
33023323
TM_Result *tmresultp,
33033324
TM_FailureData *tmfdp)
@@ -3357,29 +3378,30 @@ GetTupleForTrigger(EState *estate,
33573378
if (tmfd.traversed)
33583379
{
33593380
/*
3360-
* Recheck the tuple using EPQ. For MERGE, we leave this
3361-
* to the caller (it must do additional rechecking, and
3362-
* might end up executing a different action entirely).
3381+
* Recheck the tuple using EPQ, if requested. Otherwise,
3382+
* just return that it was concurrently updated.
33633383
*/
3364-
if (estate->es_plannedstmt->commandType == CMD_MERGE)
3384+
if (do_epq_recheck)
33653385
{
3366-
if (tmresultp)
3367-
*tmresultp = TM_Updated;
3368-
return false;
3386+
*epqslot = EvalPlanQual(epqstate,
3387+
relation,
3388+
relinfo->ri_RangeTableIndex,
3389+
oldslot);
3390+
3391+
/*
3392+
* If PlanQual failed for updated tuple - we must not
3393+
* process this tuple!
3394+
*/
3395+
if (TupIsNull(*epqslot))
3396+
{
3397+
*epqslot = NULL;
3398+
return false;
3399+
}
33693400
}
3370-
3371-
*epqslot = EvalPlanQual(epqstate,
3372-
relation,
3373-
relinfo->ri_RangeTableIndex,
3374-
oldslot);
3375-
3376-
/*
3377-
* If PlanQual failed for updated tuple - we must not
3378-
* process this tuple!
3379-
*/
3380-
if (TupIsNull(*epqslot))
3401+
else
33813402
{
3382-
*epqslot = NULL;
3403+
if (tmresultp)
3404+
*tmresultp = TM_Updated;
33833405
return false;
33843406
}
33853407
}

‎src/backend/executor/execReplication.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -670,7 +670,7 @@ ExecSimpleRelationUpdate(ResultRelInfo *resultRelInfo,
670670
resultRelInfo->ri_TrigDesc->trig_update_before_row)
671671
{
672672
if (!ExecBRUpdateTriggers(estate, epqstate, resultRelInfo,
673-
tid, NULL, slot, NULL, NULL))
673+
tid, NULL, slot, NULL, NULL, false))
674674
skip_tuple = true; /* "do nothing" */
675675
}
676676

@@ -746,7 +746,7 @@ ExecSimpleRelationDelete(ResultRelInfo *resultRelInfo,
746746
resultRelInfo->ri_TrigDesc->trig_delete_before_row)
747747
{
748748
skip_tuple = !ExecBRDeleteTriggers(estate, epqstate, resultRelInfo,
749-
tid, NULL, NULL, NULL, NULL);
749+
tid, NULL, NULL, NULL, NULL, false);
750750
}
751751

752752
if (!skip_tuple)

‎src/backend/executor/nodeModifyTable.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1474,7 +1474,8 @@ ExecDeletePrologue(ModifyTableContext *context, ResultRelInfo *resultRelInfo,
14741474

14751475
return ExecBRDeleteTriggers(context->estate, context->epqstate,
14761476
resultRelInfo, tupleid, oldtuple,
1477-
epqreturnslot, result, &context->tmfd);
1477+
epqreturnslot, result, &context->tmfd,
1478+
context->mtstate->operation == CMD_MERGE);
14781479
}
14791480

14801481
return true;
@@ -2117,7 +2118,8 @@ ExecUpdatePrologue(ModifyTableContext *context, ResultRelInfo *resultRelInfo,
21172118

21182119
return ExecBRUpdateTriggers(context->estate, context->epqstate,
21192120
resultRelInfo, tupleid, oldtuple, slot,
2120-
result, &context->tmfd);
2121+
result, &context->tmfd,
2122+
context->mtstate->operation == CMD_MERGE);
21212123
}
21222124

21232125
return true;

‎src/include/commands/trigger.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,8 @@ extern bool ExecBRDeleteTriggers(EState *estate,
213213
HeapTuple fdw_trigtuple,
214214
TupleTableSlot **epqslot,
215215
TM_Result *tmresult,
216-
TM_FailureData *tmfd);
216+
TM_FailureData *tmfd,
217+
bool is_merge_delete);
217218
extern void ExecARDeleteTriggers(EState *estate,
218219
ResultRelInfo *relinfo,
219220
ItemPointer tupleid,
@@ -235,7 +236,8 @@ extern bool ExecBRUpdateTriggers(EState *estate,
235236
HeapTuple fdw_trigtuple,
236237
TupleTableSlot *newslot,
237238
TM_Result *tmresult,
238-
TM_FailureData *tmfd);
239+
TM_FailureData *tmfd,
240+
bool is_merge_update);
239241
extern void ExecARUpdateTriggers(EState *estate,
240242
ResultRelInfo *relinfo,
241243
ResultRelInfo *src_partinfo,

‎src/test/isolation/expected/merge-match-recheck.out

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -241,19 +241,28 @@ starting permutation: update_bal1_tg merge_bal_tg c2 select1_tg c1
241241
s2: NOTICE: Update: (1,160,s1,setup) -> (1,50,s1,"setup updated by update_bal1_tg")
242242
step update_bal1_tg: UPDATE target_tg t SET balance = 50, val = t.val || ' updated by update_bal1_tg' WHERE t.key = 1;
243243
step merge_bal_tg:
244-
MERGE INTO target_tg t
245-
USING (SELECT 1 as key) s
246-
ON s.key = t.key
247-
WHEN MATCHED AND balance < 100 THEN
248-
UPDATE SET balance = balance * 2, val = t.val || ' when1'
249-
WHEN MATCHED AND balance < 200 THEN
250-
UPDATE SET balance = balance * 4, val = t.val || ' when2'
251-
WHEN MATCHED AND balance < 300 THEN
252-
UPDATE SET balance = balance * 8, val = t.val || ' when3';
244+
WITH t AS (
245+
MERGE INTO target_tg t
246+
USING (SELECT 1 as key) s
247+
ON s.key = t.key
248+
WHEN MATCHED AND balance < 100 THEN
249+
UPDATE SET balance = balance * 2, val = t.val || ' when1'
250+
WHEN MATCHED AND balance < 200 THEN
251+
UPDATE SET balance = balance * 4, val = t.val || ' when2'
252+
WHEN MATCHED AND balance < 300 THEN
253+
UPDATE SET balance = balance * 8, val = t.val || ' when3'
254+
RETURNING t.*
255+
)
256+
SELECT * FROM t;
253257
<waiting ...>
254258
step c2: COMMIT;
255259
s1: NOTICE: Update: (1,50,s1,"setup updated by update_bal1_tg") -> (1,100,s1,"setup updated by update_bal1_tg when1")
256260
step merge_bal_tg: <... completed>
261+
key|balance|status|val
262+
---+-------+------+-------------------------------------
263+
1| 100|s1 |setup updated by update_bal1_tg when1
264+
(1 row)
265+
257266
step select1_tg: SELECT * FROM target_tg;
258267
key|balance|status|val
259268
---+-------+------+-------------------------------------

‎src/test/isolation/specs/merge-match-recheck.spec

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -99,15 +99,19 @@ step "merge_bal_pa"
9999
}
100100
step "merge_bal_tg"
101101
{
102-
MERGE INTO target_tg t
103-
USING (SELECT 1 as key) s
104-
ON s.key = t.key
105-
WHEN MATCHED AND balance < 100 THEN
106-
UPDATE SET balance = balance * 2, val = t.val || ' when1'
107-
WHEN MATCHED AND balance < 200 THEN
108-
UPDATE SET balance = balance * 4, val = t.val || ' when2'
109-
WHEN MATCHED AND balance < 300 THEN
110-
UPDATE SET balance = balance * 8, val = t.val || ' when3';
102+
WITH t AS (
103+
MERGE INTO target_tg t
104+
USING (SELECT 1 as key) s
105+
ON s.key = t.key
106+
WHEN MATCHED AND balance < 100 THEN
107+
UPDATE SET balance = balance * 2, val = t.val || ' when1'
108+
WHEN MATCHED AND balance < 200 THEN
109+
UPDATE SET balance = balance * 4, val = t.val || ' when2'
110+
WHEN MATCHED AND balance < 300 THEN
111+
UPDATE SET balance = balance * 8, val = t.val || ' when3'
112+
RETURNING t.*
113+
)
114+
SELECT * FROM t;
111115
}
112116

113117
step "merge_delete"

0 commit comments

Comments
 (0)