diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 6fea82e0d4..1ad5dcb406 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -2947,8 +2947,29 @@ lmerge_matched:; case TM_SelfModified: /* - * The SQL standard disallows this for MERGE. + * The target tuple was already updated or deleted by the + * current command, or by a later command in the current + * transaction. The former case is explicitly disallowed by + * the SQL standard for MERGE, which insists that the MERGE + * join condition should not join a target row to more than + * one source row. + * + * The latter case arises if the tuple is modified by a + * command in a BEFORE trigger, or perhaps by a command in a + * volatile function used in the query. In such situations we + * should not ignore the MERGE action, but it is equally + * unsafe to proceed. We don't want to discard the original + * MERGE action while keeping the triggered actions based on + * it; and it would be no better to allow the original MERGE + * action while discarding the updates that it triggered. So + * throwing an error is the only safe course. */ + if (context->tmfd.cmax != estate->es_output_cid) + ereport(ERROR, + (errcode(ERRCODE_TRIGGERED_DATA_CHANGE_VIOLATION), + errmsg("tuple to be updated or deleted was already modified by an operation triggered by the current command"), + errhint("Consider using an AFTER trigger instead of a BEFORE trigger to propagate changes to other rows."))); + if (TransactionIdIsCurrentTransactionId(context->tmfd.xmax)) ereport(ERROR, (errcode(ERRCODE_CARDINALITY_VIOLATION), @@ -2956,6 +2977,7 @@ lmerge_matched:; errmsg("%s command cannot affect row a second time", "MERGE"), errhint("Ensure that not more than one source row matches any one target row."))); + /* This shouldn't happen */ elog(ERROR, "attempted to update or delete invisible tuple"); break; @@ -3064,19 +3086,27 @@ lmerge_matched:; /* * This can be reached when following an update * chain from a tuple updated by another session, - * reaching a tuple that was already updated in - * this transaction. If previously modified by - * this command, ignore the redundant update, - * otherwise error out. - * - * See also response to TM_SelfModified in - * ExecUpdate(). + * reaching a tuple that was already updated or + * deleted by the current command, or by a later + * command in the current transaction. As above, + * this should always be treated as an error. */ if (context->tmfd.cmax != estate->es_output_cid) ereport(ERROR, (errcode(ERRCODE_TRIGGERED_DATA_CHANGE_VIOLATION), errmsg("tuple to be updated or deleted was already modified by an operation triggered by the current command"), errhint("Consider using an AFTER trigger instead of a BEFORE trigger to propagate changes to other rows."))); + + if (TransactionIdIsCurrentTransactionId(context->tmfd.xmax)) + ereport(ERROR, + (errcode(ERRCODE_CARDINALITY_VIOLATION), + /* translator: %s is a SQL command name */ + errmsg("%s command cannot affect row a second time", + "MERGE"), + errhint("Ensure that not more than one source row matches any one target row."))); + + /* This shouldn't happen */ + elog(ERROR, "attempted to update or delete invisible tuple"); return false; default: diff --git a/src/test/isolation/expected/merge-update.out b/src/test/isolation/expected/merge-update.out index 55b1f908fd..f5f7e3ba19 100644 --- a/src/test/isolation/expected/merge-update.out +++ b/src/test/isolation/expected/merge-update.out @@ -48,6 +48,27 @@ key|val step c2: COMMIT; +starting permutation: pa_merge1 c1 pa_merge2c_dup a2 +step pa_merge1: + MERGE INTO pa_target t + USING (SELECT 1 as key, 'pa_merge1' as val) s + ON s.key = t.key + WHEN NOT MATCHED THEN + INSERT VALUES (s.key, s.val) + WHEN MATCHED THEN + UPDATE set val = t.val || ' updated by ' || s.val; + +step c1: COMMIT; +step pa_merge2c_dup: + MERGE INTO pa_target t + USING (VALUES (1), (1)) v(a) + ON t.key = v.a + WHEN MATCHED THEN + UPDATE set val = t.val || ' updated by pa_merge2c_dup'; -- should fail + +ERROR: MERGE command cannot affect row a second time +step a2: ABORT; + starting permutation: merge1 merge2a c1 select2 c2 step merge1: MERGE INTO target t @@ -312,3 +333,25 @@ key|val (2 rows) step c2: COMMIT; + +starting permutation: pa_merge1 pa_merge2c_dup c1 a2 +step pa_merge1: + MERGE INTO pa_target t + USING (SELECT 1 as key, 'pa_merge1' as val) s + ON s.key = t.key + WHEN NOT MATCHED THEN + INSERT VALUES (s.key, s.val) + WHEN MATCHED THEN + UPDATE set val = t.val || ' updated by ' || s.val; + +step pa_merge2c_dup: + MERGE INTO pa_target t + USING (VALUES (1), (1)) v(a) + ON t.key = v.a + WHEN MATCHED THEN + UPDATE set val = t.val || ' updated by pa_merge2c_dup'; -- should fail + +step c1: COMMIT; +step pa_merge2c_dup: <... completed> +ERROR: MERGE command cannot affect row a second time +step a2: ABORT; diff --git a/src/test/isolation/specs/merge-update.spec b/src/test/isolation/specs/merge-update.spec index e8d01666fe..3ccd466449 100644 --- a/src/test/isolation/specs/merge-update.spec +++ b/src/test/isolation/specs/merge-update.spec @@ -4,6 +4,7 @@ # 1. UPDATEs of PKs that change the join in the ON clause # 2. UPDATEs with WHEN conditions that would fail after concurrent update # 3. UPDATEs with extra ON conditions that would fail after concurrent update +# 4. UPDATEs with duplicate source rows setup { @@ -134,15 +135,26 @@ step "pa_merge2b_when" WHEN MATCHED AND t.val like 'initial%' THEN UPDATE set key = t.key + 1, val = t.val || ' updated by ' || s.val; } +# Duplicate source row should fail +step "pa_merge2c_dup" +{ + MERGE INTO pa_target t + USING (VALUES (1), (1)) v(a) + ON t.key = v.a + WHEN MATCHED THEN + UPDATE set val = t.val || ' updated by pa_merge2c_dup'; -- should fail +} step "select2" { SELECT * FROM target; } step "pa_select2" { SELECT * FROM pa_target; } step "c2" { COMMIT; } +step "a2" { ABORT; } # Basic effects permutation "merge1" "c1" "select2" "c2" # One after the other, no concurrency permutation "merge1" "c1" "merge2a" "select2" "c2" +permutation "pa_merge1" "c1" "pa_merge2c_dup" "a2" # Now with concurrency permutation "merge1" "merge2a" "c1" "select2" "c2" @@ -154,3 +166,4 @@ permutation "pa_merge2" "pa_merge2a" "c1" "pa_select2" "c2" # fails permutation "pa_merge2" "c1" "pa_merge2a" "pa_select2" "c2" # succeeds permutation "pa_merge3" "pa_merge2b_when" "c1" "pa_select2" "c2" # WHEN not satisfied by updated tuple permutation "pa_merge1" "pa_merge2b_when" "c1" "pa_select2" "c2" # WHEN satisfied by updated tuple +permutation "pa_merge1" "pa_merge2c_dup" "c1" "a2" diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out index 723e8b7166..4dd95e2680 100644 --- a/src/test/regress/expected/triggers.out +++ b/src/test/regress/expected/triggers.out @@ -1745,6 +1745,10 @@ select * from parent; select * from child; update parent set val1 = 'b' where aid = 1; -- should fail ERROR: tuple to be updated was already modified by an operation triggered by the current command HINT: Consider using an AFTER trigger instead of a BEFORE trigger to propagate changes to other rows. +merge into parent p using (values (1)) as v(id) on p.aid = v.id + when matched then update set val1 = 'b'; -- should fail +ERROR: tuple to be updated or deleted was already modified by an operation triggered by the current command +HINT: Consider using an AFTER trigger instead of a BEFORE trigger to propagate changes to other rows. select * from parent; select * from child; aid | val1 | val2 | val3 | val4 | bcnt -----+------+------+------+------+------ @@ -1759,6 +1763,10 @@ select * from parent; select * from child; delete from parent where aid = 1; -- should fail ERROR: tuple to be deleted was already modified by an operation triggered by the current command HINT: Consider using an AFTER trigger instead of a BEFORE trigger to propagate changes to other rows. +merge into parent p using (values (1)) as v(id) on p.aid = v.id + when matched then delete; -- should fail +ERROR: tuple to be updated or deleted was already modified by an operation triggered by the current command +HINT: Consider using an AFTER trigger instead of a BEFORE trigger to propagate changes to other rows. select * from parent; select * from child; aid | val1 | val2 | val3 | val4 | bcnt -----+------+------+------+------+------ diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql index 46795a9c78..6c9e066397 100644 --- a/src/test/regress/sql/triggers.sql +++ b/src/test/regress/sql/triggers.sql @@ -1186,9 +1186,13 @@ insert into child values (10, 1, 'b'); select * from parent; select * from child; update parent set val1 = 'b' where aid = 1; -- should fail +merge into parent p using (values (1)) as v(id) on p.aid = v.id + when matched then update set val1 = 'b'; -- should fail select * from parent; select * from child; delete from parent where aid = 1; -- should fail +merge into parent p using (values (1)) as v(id) on p.aid = v.id + when matched then delete; -- should fail select * from parent; select * from child; -- replace the trigger function with one that restarts the deletion after