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

Commit5022ff2

Browse files
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 withBEFORE ROW triggers, and a concurrent UPDATE or DELETE happens, themerge code would fail (crashing in the case of an UPDATE action, andpotentially executing the wrong action for a DELETE action).This is the same issue that9321c79 attempted to fix, except nowfor a MERGE inside a CTE. As noted in9321c79, what needs to happenis for the trigger code to exit early, returning the TM_Result andTM_FailureData information to the merge code, if a concurrentmodification is detected, rather than attempting to do an EPQrecheck. The merge code will then do its own rechecking, and rescanthe action list, potentially executing a different action in light ofthe concurrent update. In particular, the trigger code must never callExecGetUpdateNewTuple() for MERGE, since that is bound to fail becauseMERGE has its own per-action projection information.Commit9321c79 did this using estate->es_plannedstmt->commandTypein the trigger code to detect that a MERGE was being executed, whichis fine for a plain MERGE command, but does not work for a MERGEinside a CTE. Fix by passing that information to the trigger code asan additional parameter passed to ExecBRUpdateTriggers() andExecBRDeleteTriggers().Back-patch as far as v17 only, since MERGE cannot appear inside a CTEprior to that. Additionally, take care to preserve the trigger ABI inv17 (though not in v18, which is still in beta).Bug: #18986Reported-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.orgBackpatch-through: 17
1 parent62c3b4c commit5022ff2

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
ItemPointertid,
8181
LockTupleModelockmode,
8282
TupleTableSlot*oldslot,
83+
booldo_epq_recheck,
8384
TupleTableSlot**epqslot,
8485
TM_Result*tmresultp,
8586
TM_FailureData*tmfdp);
@@ -2693,7 +2694,8 @@ ExecBRDeleteTriggers(EState *estate, EPQState *epqstate,
26932694
HeapTuplefdw_trigtuple,
26942695
TupleTableSlot**epqslot,
26952696
TM_Result*tmresult,
2696-
TM_FailureData*tmfd)
2697+
TM_FailureData*tmfd,
2698+
boolis_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
HeapTuplefdw_trigtuple,
29452956
TupleTableSlot*newslot,
29462957
TM_Result*tmresult,
2947-
TM_FailureData*tmfd)
2958+
TM_FailureData*tmfd,
2959+
boolis_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
ItemPointertid,
32993319
LockTupleModelockmode,
33003320
TupleTableSlot*oldslot,
3321+
booldo_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
returnExecBRDeleteTriggers(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
returnExecBRUpdateTriggers(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
HeapTuplefdw_trigtuple,
214214
TupleTableSlot**epqslot,
215215
TM_Result*tmresult,
216-
TM_FailureData*tmfd);
216+
TM_FailureData*tmfd,
217+
boolis_merge_delete);
217218
externvoidExecARDeleteTriggers(EState*estate,
218219
ResultRelInfo*relinfo,
219220
ItemPointertupleid,
@@ -235,7 +236,8 @@ extern bool ExecBRUpdateTriggers(EState *estate,
235236
HeapTuplefdw_trigtuple,
236237
TupleTableSlot*newslot,
237238
TM_Result*tmresult,
238-
TM_FailureData*tmfd);
239+
TM_FailureData*tmfd,
240+
boolis_merge_update);
239241
externvoidExecARUpdateTriggers(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-
MERGEINTOtarget_tgt
103-
USING (SELECT1askey)s
104-
ONs.key=t.key
105-
WHENMATCHEDANDbalance<100THEN
106-
UPDATESETbalance=balance*2,val=t.val||' when1'
107-
WHENMATCHEDANDbalance<200THEN
108-
UPDATESETbalance=balance*4,val=t.val||' when2'
109-
WHENMATCHEDANDbalance<300THEN
110-
UPDATESETbalance=balance*8,val=t.val||' when3';
102+
WITHtAS (
103+
MERGEINTOtarget_tgt
104+
USING (SELECT1askey)s
105+
ONs.key=t.key
106+
WHENMATCHEDANDbalance<100THEN
107+
UPDATESETbalance=balance*2,val=t.val||' when1'
108+
WHENMATCHEDANDbalance<200THEN
109+
UPDATESETbalance=balance*4,val=t.val||' when2'
110+
WHENMATCHEDANDbalance<300THEN
111+
UPDATESETbalance=balance*8,val=t.val||' when3'
112+
RETURNINGt.*
113+
)
114+
SELECT*FROMt;
111115
}
112116

113117
step"merge_delete"

0 commit comments

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