Skip to content

Commit dd73d10

Browse files
deanrasheeddeanrasheed
committed
Fix handling of self-modified tuples in MERGE.
When an UPDATE or DELETE action in MERGE returns TM_SelfModified, there are 2 possible causes: 1). The target tuple was already updated or deleted by the current command. This can happen if the target row joins to more than one source row, and the SQL standard explicitly says that this must be an error. 2). The target tuple was already updated or deleted by a later command in the current transaction. This can happen if the tuple is modified by a BEFORE trigger or a volatile function used in the query, and should be an error for the same reason that it is in a plain UPDATE or DELETE command. In MERGE's primary error handling block, it failed to check for (2), causing it to return a misleading error message in such cases. In the secondary error handling block, following a concurrent update from another session, it failed to check for (1), causing it to silently ignore target rows joined to more than one source row, instead of reporting an error. Fix this, and add tests for both of these cases. Per report from Wenjiang Zhang. Back-patch to v15, where MERGE was introduced. Discussion: https://postgr.es/m/tencent_41DE0FF443FE14B94A5898D373792109E408%40qq.com
1 parent c46817e commit dd73d10

File tree

5 files changed

+106
-8
lines changed

5 files changed

+106
-8
lines changed

‎src/backend/executor/nodeModifyTable.c

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2966,15 +2966,37 @@ ExecMergeMatched(ModifyTableContext *context, ResultRelInfo *resultRelInfo,
29662966
case TM_SelfModified:
29672967

29682968
/*
2969-
* The SQL standard disallows this for MERGE.
2969+
* The target tuple was already updated or deleted by the
2970+
* current command, or by a later command in the current
2971+
* transaction. The former case is explicitly disallowed by
2972+
* the SQL standard for MERGE, which insists that the MERGE
2973+
* join condition should not join a target row to more than
2974+
* one source row.
2975+
*
2976+
* The latter case arises if the tuple is modified by a
2977+
* command in a BEFORE trigger, or perhaps by a command in a
2978+
* volatile function used in the query. In such situations we
2979+
* should not ignore the MERGE action, but it is equally
2980+
* unsafe to proceed. We don't want to discard the original
2981+
* MERGE action while keeping the triggered actions based on
2982+
* it; and it would be no better to allow the original MERGE
2983+
* action while discarding the updates that it triggered. So
2984+
* throwing an error is the only safe course.
29702985
*/
2986+
if (context->tmfd.cmax != estate->es_output_cid)
2987+
ereport(ERROR,
2988+
(errcode(ERRCODE_TRIGGERED_DATA_CHANGE_VIOLATION),
2989+
errmsg("tuple to be updated or deleted was already modified by an operation triggered by the current command"),
2990+
errhint("Consider using an AFTER trigger instead of a BEFORE trigger to propagate changes to other rows.")));
2991+
29712992
if (TransactionIdIsCurrentTransactionId(context->tmfd.xmax))
29722993
ereport(ERROR,
29732994
(errcode(ERRCODE_CARDINALITY_VIOLATION),
29742995
/* translator: %s is a SQL command name */
29752996
errmsg("%s command cannot affect row a second time",
29762997
"MERGE"),
29772998
errhint("Ensure that not more than one source row matches any one target row.")));
2999+
29783000
/* This shouldn't happen */
29793001
elog(ERROR, "attempted to update or delete invisible tuple");
29803002
break;
@@ -3083,19 +3105,27 @@ ExecMergeMatched(ModifyTableContext *context, ResultRelInfo *resultRelInfo,
30833105
/*
30843106
* This can be reached when following an update
30853107
* chain from a tuple updated by another session,
3086-
* reaching a tuple that was already updated in
3087-
* this transaction. If previously modified by
3088-
* this command, ignore the redundant update,
3089-
* otherwise error out.
3090-
*
3091-
* See also response to TM_SelfModified in
3092-
* ExecUpdate().
3108+
* reaching a tuple that was already updated or
3109+
* deleted by the current command, or by a later
3110+
* command in the current transaction. As above,
3111+
* this should always be treated as an error.
30933112
*/
30943113
if (context->tmfd.cmax != estate->es_output_cid)
30953114
ereport(ERROR,
30963115
(errcode(ERRCODE_TRIGGERED_DATA_CHANGE_VIOLATION),
30973116
errmsg("tuple to be updated or deleted was already modified by an operation triggered by the current command"),
30983117
errhint("Consider using an AFTER trigger instead of a BEFORE trigger to propagate changes to other rows.")));
3118+
3119+
if (TransactionIdIsCurrentTransactionId(context->tmfd.xmax))
3120+
ereport(ERROR,
3121+
(errcode(ERRCODE_CARDINALITY_VIOLATION),
3122+
/* translator: %s is a SQL command name */
3123+
errmsg("%s command cannot affect row a second time",
3124+
"MERGE"),
3125+
errhint("Ensure that not more than one source row matches any one target row.")));
3126+
3127+
/* This shouldn't happen */
3128+
elog(ERROR, "attempted to update or delete invisible tuple");
30993129
return false;
31003130

31013131
default:

‎src/test/isolation/expected/merge-update.out

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,27 @@ key|val
4848

4949
step c2: COMMIT;
5050

51+
starting permutation: pa_merge1 c1 pa_merge2c_dup a2
52+
step pa_merge1:
53+
MERGE INTO pa_target t
54+
USING (SELECT 1 as key, 'pa_merge1' as val) s
55+
ON s.key = t.key
56+
WHEN NOT MATCHED THEN
57+
INSERT VALUES (s.key, s.val)
58+
WHEN MATCHED THEN
59+
UPDATE set val = t.val || ' updated by ' || s.val;
60+
61+
step c1: COMMIT;
62+
step pa_merge2c_dup:
63+
MERGE INTO pa_target t
64+
USING (VALUES (1), (1)) v(a)
65+
ON t.key = v.a
66+
WHEN MATCHED THEN
67+
UPDATE set val = t.val || ' updated by pa_merge2c_dup'; -- should fail
68+
69+
ERROR: MERGE command cannot affect row a second time
70+
step a2: ABORT;
71+
5172
starting permutation: merge1 merge2a c1 select2 c2
5273
step merge1:
5374
MERGE INTO target t
@@ -312,3 +333,25 @@ key|val
312333
(2 rows)
313334

314335
step c2: COMMIT;
336+
337+
starting permutation: pa_merge1 pa_merge2c_dup c1 a2
338+
step pa_merge1:
339+
MERGE INTO pa_target t
340+
USING (SELECT 1 as key, 'pa_merge1' as val) s
341+
ON s.key = t.key
342+
WHEN NOT MATCHED THEN
343+
INSERT VALUES (s.key, s.val)
344+
WHEN MATCHED THEN
345+
UPDATE set val = t.val || ' updated by ' || s.val;
346+
347+
step pa_merge2c_dup:
348+
MERGE INTO pa_target t
349+
USING (VALUES (1), (1)) v(a)
350+
ON t.key = v.a
351+
WHEN MATCHED THEN
352+
UPDATE set val = t.val || ' updated by pa_merge2c_dup'; -- should fail
353+
<waiting ...>
354+
step c1: COMMIT;
355+
step pa_merge2c_dup: <... completed>
356+
ERROR: MERGE command cannot affect row a second time
357+
step a2: ABORT;

‎src/test/isolation/specs/merge-update.spec

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
# 1. UPDATEs of PKs that change the join in the ON clause
55
# 2. UPDATEs with WHEN conditions that would fail after concurrent update
66
# 3. UPDATEs with extra ON conditions that would fail after concurrent update
7+
# 4. UPDATEs with duplicate source rows
78

89
setup
910
{
@@ -134,15 +135,26 @@ step "pa_merge2b_when"
134135
WHEN MATCHED AND t.val like 'initial%' THEN
135136
UPDATE set key = t.key + 1, val = t.val || ' updated by ' || s.val;
136137
}
138+
# Duplicate source row should fail
139+
step "pa_merge2c_dup"
140+
{
141+
MERGE INTO pa_target t
142+
USING (VALUES (1), (1)) v(a)
143+
ON t.key = v.a
144+
WHEN MATCHED THEN
145+
UPDATE set val = t.val || ' updated by pa_merge2c_dup'; -- should fail
146+
}
137147
step "select2" { SELECT * FROM target; }
138148
step "pa_select2" { SELECT * FROM pa_target; }
139149
step "c2" { COMMIT; }
150+
step "a2" { ABORT; }
140151

141152
# Basic effects
142153
permutation "merge1" "c1" "select2" "c2"
143154

144155
# One after the other, no concurrency
145156
permutation "merge1" "c1" "merge2a" "select2" "c2"
157+
permutation "pa_merge1" "c1" "pa_merge2c_dup" "a2"
146158

147159
# Now with concurrency
148160
permutation "merge1" "merge2a" "c1" "select2" "c2"
@@ -154,3 +166,4 @@ permutation "pa_merge2" "pa_merge2a" "c1" "pa_select2" "c2" # fails
154166
permutation "pa_merge2" "c1" "pa_merge2a" "pa_select2" "c2" # succeeds
155167
permutation "pa_merge3" "pa_merge2b_when" "c1" "pa_select2" "c2" # WHEN not satisfied by updated tuple
156168
permutation "pa_merge1" "pa_merge2b_when" "c1" "pa_select2" "c2" # WHEN satisfied by updated tuple
169+
permutation "pa_merge1" "pa_merge2c_dup" "c1" "a2"

‎src/test/regress/expected/triggers.out

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1745,6 +1745,10 @@ select * from parent; select * from child;
17451745
update parent set val1 = 'b' where aid = 1; -- should fail
17461746
ERROR: tuple to be updated was already modified by an operation triggered by the current command
17471747
HINT: Consider using an AFTER trigger instead of a BEFORE trigger to propagate changes to other rows.
1748+
merge into parent p using (values (1)) as v(id) on p.aid = v.id
1749+
when matched then update set val1 = 'b'; -- should fail
1750+
ERROR: tuple to be updated or deleted was already modified by an operation triggered by the current command
1751+
HINT: Consider using an AFTER trigger instead of a BEFORE trigger to propagate changes to other rows.
17481752
select * from parent; select * from child;
17491753
aid | val1 | val2 | val3 | val4 | bcnt
17501754
-----+------+------+------+------+------
@@ -1759,6 +1763,10 @@ select * from parent; select * from child;
17591763
delete from parent where aid = 1; -- should fail
17601764
ERROR: tuple to be deleted was already modified by an operation triggered by the current command
17611765
HINT: Consider using an AFTER trigger instead of a BEFORE trigger to propagate changes to other rows.
1766+
merge into parent p using (values (1)) as v(id) on p.aid = v.id
1767+
when matched then delete; -- should fail
1768+
ERROR: tuple to be updated or deleted was already modified by an operation triggered by the current command
1769+
HINT: Consider using an AFTER trigger instead of a BEFORE trigger to propagate changes to other rows.
17621770
select * from parent; select * from child;
17631771
aid | val1 | val2 | val3 | val4 | bcnt
17641772
-----+------+------+------+------+------

‎src/test/regress/sql/triggers.sql

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1186,9 +1186,13 @@ insert into child values (10, 1, 'b');
11861186
select * from parent; select * from child;
11871187

11881188
update parent set val1 = 'b' where aid = 1; -- should fail
1189+
merge into parent p using (values (1)) as v(id) on p.aid = v.id
1190+
when matched then update set val1 = 'b'; -- should fail
11891191
select * from parent; select * from child;
11901192

11911193
delete from parent where aid = 1; -- should fail
1194+
merge into parent p using (values (1)) as v(id) on p.aid = v.id
1195+
when matched then delete; -- should fail
11921196
select * from parent; select * from child;
11931197

11941198
-- replace the trigger function with one that restarts the deletion after

0 commit comments

Comments
 (0)