diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index f2231b0aed9..3b790e00599 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -401,6 +401,23 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("TRUNCATE triggers with transition tables are not supported"))); + /* + * We currently don't allow multi-event triggers ("INSERT OR + * UPDATE") with transition tables, because it's not clear how to + * handle INSERT ... ON CONFLICT statements which can fire both + * INSERT and UPDATE triggers. We show the inserted tuples to + * INSERT triggers and the updated tuples to UPDATE triggers, but + * it's not yet clear what INSERT OR UPDATE trigger should see. + * This restriction could be lifted if we can decide on the right + * semantics in a later release. + */ + if (((TRIGGER_FOR_INSERT(tgtype) ? 1 : 0) + + (TRIGGER_FOR_UPDATE(tgtype) ? 1 : 0) + + (TRIGGER_FOR_DELETE(tgtype) ? 1 : 0)) != 1) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("Transition tables cannot be specified for triggers with more than one event"))); + if (tt->isNew) { if (!(TRIGGER_FOR_INSERT(tgtype) || @@ -2109,8 +2126,10 @@ MakeTransitionCaptureState(TriggerDesc *trigdesc) CurrentResourceOwner = TopTransactionResourceOwner; if (trigdesc->trig_delete_old_table || trigdesc->trig_update_old_table) state->tcs_old_tuplestore = tuplestore_begin_heap(false, false, work_mem); - if (trigdesc->trig_insert_new_table || trigdesc->trig_update_new_table) - state->tcs_new_tuplestore = tuplestore_begin_heap(false, false, work_mem); + if (trigdesc->trig_insert_new_table) + state->tcs_insert_tuplestore = tuplestore_begin_heap(false, false, work_mem); + if (trigdesc->trig_update_new_table) + state->tcs_update_tuplestore = tuplestore_begin_heap(false, false, work_mem); } PG_CATCH(); { @@ -2128,8 +2147,10 @@ MakeTransitionCaptureState(TriggerDesc *trigdesc) void DestroyTransitionCaptureState(TransitionCaptureState *tcs) { - if (tcs->tcs_new_tuplestore != NULL) - tuplestore_end(tcs->tcs_new_tuplestore); + if (tcs->tcs_insert_tuplestore != NULL) + tuplestore_end(tcs->tcs_insert_tuplestore); + if (tcs->tcs_update_tuplestore != NULL) + tuplestore_end(tcs->tcs_update_tuplestore); if (tcs->tcs_old_tuplestore != NULL) tuplestore_end(tcs->tcs_old_tuplestore); pfree(tcs); @@ -3975,7 +3996,29 @@ AfterTriggerExecute(AfterTriggerEvent event, if (LocTriggerData.tg_trigger->tgoldtable) LocTriggerData.tg_oldtable = transition_capture->tcs_old_tuplestore; if (LocTriggerData.tg_trigger->tgnewtable) - LocTriggerData.tg_newtable = transition_capture->tcs_new_tuplestore; + { + /* + * Currently a trigger with transition tables may only be defined + * for a single event type (here AFTER INSERT or AFTER UPDATE, but + * not AFTER INSERT OR ...). + */ + Assert((TRIGGER_FOR_INSERT(LocTriggerData.tg_trigger->tgtype) != 0) ^ + (TRIGGER_FOR_UPDATE(LocTriggerData.tg_trigger->tgtype) != 0)); + + /* + * Show either the insert or update new tuple images, depending on + * which event type the trigger was registered for. A single + * statement may have produced both in the case of INSERT ... ON + * CONFLICT ... DO UPDATE, and in that case the event determines + * which tuplestore the trigger sees as the NEW TABLE. + */ + if (TRIGGER_FOR_INSERT(LocTriggerData.tg_trigger->tgtype)) + LocTriggerData.tg_newtable = + transition_capture->tcs_insert_tuplestore; + else + LocTriggerData.tg_newtable = + transition_capture->tcs_update_tuplestore; + } } /* @@ -5243,7 +5286,10 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo, Tuplestorestate *new_tuplestore; Assert(newtup != NULL); - new_tuplestore = transition_capture->tcs_new_tuplestore; + if (event == TRIGGER_EVENT_INSERT) + new_tuplestore = transition_capture->tcs_insert_tuplestore; + else + new_tuplestore = transition_capture->tcs_update_tuplestore; if (original_insert_tuple != NULL) tuplestore_puttuple(new_tuplestore, original_insert_tuple); diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index 0a98fbfdf54..24586439de9 100644 --- a/src/backend/executor/execReplication.c +++ b/src/backend/executor/execReplication.c @@ -406,6 +406,12 @@ ExecSimpleRelationInsert(EState *estate, TupleTableSlot *slot) ExecARInsertTriggers(estate, resultRelInfo, tuple, recheckIndexes, NULL); + /* + * XXX we should in theory pass a TransitionCaptureState object to the + * above to capture transition tuples, but after statement triggers + * don't actually get fired by replication yet anyway + */ + list_free(recheckIndexes); } } diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h index 5ce57966245..ac655521c67 100644 --- a/src/include/commands/trigger.h +++ b/src/include/commands/trigger.h @@ -73,9 +73,17 @@ typedef struct TransitionCaptureState */ HeapTuple tcs_original_insert_tuple; - /* The tuplestores backing the transition tables. */ - Tuplestorestate *tcs_old_tuplestore; - Tuplestorestate *tcs_new_tuplestore; + /* + * The tuplestores backing the transition tables. We use separate + * tuplestores for INSERT and UPDATE, because INSERT ... ON CONFLICT + * ... DO UPDATE causes INSERT and UPDATE triggers to fire and needs a way + * to keep track of the new tuple images resulting from the two cases + * separately. We only need a single old image tuplestore, because there + * is no statement that can both update and delete at the same time. + */ + Tuplestorestate *tcs_old_tuplestore; /* for DELETE and UPDATE old images */ + Tuplestorestate *tcs_insert_tuplestore; /* for INSERT new images */ + Tuplestorestate *tcs_update_tuplestore; /* for UPDATE new images */ } TransitionCaptureState; /* diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out index 7ebbde60d30..f74fabc7603 100644 --- a/src/test/regress/expected/plpgsql.out +++ b/src/test/regress/expected/plpgsql.out @@ -5847,8 +5847,13 @@ AS $$ RETURN NULL; END; $$; -CREATE TRIGGER transition_table_level2_ri_child_insupd_trigger - AFTER INSERT OR UPDATE ON transition_table_level2 +CREATE TRIGGER transition_table_level2_ri_child_ins_trigger + AFTER INSERT ON transition_table_level2 + REFERENCING NEW TABLE AS i + FOR EACH STATEMENT EXECUTE PROCEDURE + transition_table_level2_ri_child_insupd_func(); +CREATE TRIGGER transition_table_level2_ri_child_upd_trigger + AFTER UPDATE ON transition_table_level2 REFERENCING NEW TABLE AS i FOR EACH STATEMENT EXECUTE PROCEDURE transition_table_level2_ri_child_insupd_func(); diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out index 0261d66e5c5..aaee30219ad 100644 --- a/src/test/regress/expected/triggers.out +++ b/src/test/regress/expected/triggers.out @@ -2214,6 +2214,44 @@ NOTICE: trigger = table2_trig, new table = ("hello world") NOTICE: trigger = table1_trig, new table = (42) drop table table1; drop table table2; +-- +-- Verify behavior of INSERT ... ON CONFLICT DO UPDATE ... with +-- transition tables. +-- +create table my_table (a int primary key, b text); +create trigger my_table_insert_trig + after insert on my_table referencing new table as new_table + for each statement execute procedure dump_insert(); +create trigger my_table_update_trig + after update on my_table referencing old table as old_table new table as new_table + for each statement execute procedure dump_update(); +-- inserts only +insert into my_table values (1, 'AAA'), (2, 'BBB') + on conflict (a) do + update set b = my_table.b || ':' || excluded.b; +NOTICE: trigger = my_table_update_trig, old table = , new table = +NOTICE: trigger = my_table_insert_trig, new table = (1,AAA), (2,BBB) +-- mixture of inserts and updates +insert into my_table values (1, 'AAA'), (2, 'BBB'), (3, 'CCC'), (4, 'DDD') + on conflict (a) do + update set b = my_table.b || ':' || excluded.b; +NOTICE: trigger = my_table_update_trig, old table = (1,AAA), (2,BBB), new table = (1,AAA:AAA), (2,BBB:BBB) +NOTICE: trigger = my_table_insert_trig, new table = (3,CCC), (4,DDD) +-- updates only +insert into my_table values (3, 'CCC'), (4, 'DDD') + on conflict (a) do + update set b = my_table.b || ':' || excluded.b; +NOTICE: trigger = my_table_update_trig, old table = (3,CCC), (4,DDD), new table = (3,CCC:CCC), (4,DDD:DDD) +NOTICE: trigger = my_table_insert_trig, new table = +-- +-- Verify that you can't create a trigger with transition tables for +-- more than one event. +-- +create trigger my_table_multievent_trig + after insert or update on my_table referencing new table as new_table + for each statement execute procedure dump_insert(); +ERROR: Transition tables cannot be specified for triggers with more than one event +drop table my_table; -- cleanup drop function dump_insert(); drop function dump_update(); diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql index 60d1d38e346..f2881de2bf7 100644 --- a/src/test/regress/sql/plpgsql.sql +++ b/src/test/regress/sql/plpgsql.sql @@ -4639,8 +4639,14 @@ AS $$ END; $$; -CREATE TRIGGER transition_table_level2_ri_child_insupd_trigger - AFTER INSERT OR UPDATE ON transition_table_level2 +CREATE TRIGGER transition_table_level2_ri_child_ins_trigger + AFTER INSERT ON transition_table_level2 + REFERENCING NEW TABLE AS i + FOR EACH STATEMENT EXECUTE PROCEDURE + transition_table_level2_ri_child_insupd_func(); + +CREATE TRIGGER transition_table_level2_ri_child_upd_trigger + AFTER UPDATE ON transition_table_level2 REFERENCING NEW TABLE AS i FOR EACH STATEMENT EXECUTE PROCEDURE transition_table_level2_ri_child_insupd_func(); diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql index 128126a0f1a..659a5a1422b 100644 --- a/src/test/regress/sql/triggers.sql +++ b/src/test/regress/sql/triggers.sql @@ -1725,6 +1725,45 @@ with wcte as (insert into table1 values (42)) drop table table1; drop table table2; +-- +-- Verify behavior of INSERT ... ON CONFLICT DO UPDATE ... with +-- transition tables. +-- + +create table my_table (a int primary key, b text); +create trigger my_table_insert_trig + after insert on my_table referencing new table as new_table + for each statement execute procedure dump_insert(); +create trigger my_table_update_trig + after update on my_table referencing old table as old_table new table as new_table + for each statement execute procedure dump_update(); + +-- inserts only +insert into my_table values (1, 'AAA'), (2, 'BBB') + on conflict (a) do + update set b = my_table.b || ':' || excluded.b; + +-- mixture of inserts and updates +insert into my_table values (1, 'AAA'), (2, 'BBB'), (3, 'CCC'), (4, 'DDD') + on conflict (a) do + update set b = my_table.b || ':' || excluded.b; + +-- updates only +insert into my_table values (3, 'CCC'), (4, 'DDD') + on conflict (a) do + update set b = my_table.b || ':' || excluded.b; + +-- +-- Verify that you can't create a trigger with transition tables for +-- more than one event. +-- + +create trigger my_table_multievent_trig + after insert or update on my_table referencing new table as new_table + for each statement execute procedure dump_insert(); + +drop table my_table; + -- cleanup drop function dump_insert(); drop function dump_update();