Transition tables vs ON CONFLICT
Started by Thomas Munroover 8 years ago5 messages
[Moving this to its own thread, for earlier discussion see the
transition-tables-vs-wCTE thread[1]/messages/by-id/CAEepm=3HZY+2Vr5P3pvVYfKLrwhPWT6vGLtBOeCh6K5Cwb8L7w@mail.gmail.com.]
On Wed, Jun 7, 2017 at 10:47 AM, Peter Geoghegan <pg@bowt.ie> wrote:
I suppose you'll need two tuplestores for the ON CONFLICT DO UPDATE
case -- one for updated tuples, and the other for inserted tuples.
On Fri, Jun 9, 2017 at 11:28 AM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
I discussed this off-list with Andrew Gierth and we came up with a
fourth way: Use separate insert and update tuplestores (as originally
suggested by Peter) and use the <trigger event> (INSERT, UPDATE) to
decide which one a trigger should see, as described in option 2 above,
but disallow INSERT OR UPDATE triggers with transition tables so that
we don't have to choose any of the surprising behaviours described
above. Triggers with multiple <trigger event>s are a PostgreSQL
extension, so by not allowing them with transition tables we don't
reduce our compliance. If you want to be invoked twice when you run
ON CONFLICT statements (like option 3 above) then you'll need to
create two triggers, one for INSERT and the other for UPDATE, and each
will see only the transition tuples resulting from inserts or updates
respectively.The door is still open for us to allow INSERT OR UPDATE with
transition tables in future releases if someone can figure out what
that should do.
Here is a patch implementing the above. It should be applied on top
of transition-tuples-from-wctes-v2.patch[2]/messages/by-id/CAEepm=2ZQ+mujsvWXhOqaNxpc2-0hDev6q7a+XrbOn2=cr7=0A@mail.gmail.com.
This is patch 3 of a stack of 3 patches addressing currently known
problems with transition tables.
[1]: /messages/by-id/CAEepm=3HZY+2Vr5P3pvVYfKLrwhPWT6vGLtBOeCh6K5Cwb8L7w@mail.gmail.com
[2]: /messages/by-id/CAEepm=2ZQ+mujsvWXhOqaNxpc2-0hDev6q7a+XrbOn2=cr7=0A@mail.gmail.com
--
Thomas Munro
http://www.enterprisedb.com
Attachments:
transition-tuples-from-on-conflict-v1.patchapplication/octet-stream; name=transition-tuples-from-on-conflict-v1.patchDownload
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 = <NULL>, new table = <NULL>
+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 = <NULL>
+--
+-- 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();
Re: Transition tables vs ON CONFLICT
On Fri, Jun 9, 2017 at 4:10 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
On Wed, Jun 7, 2017 at 10:47 AM, Peter Geoghegan <pg@bowt.ie> wrote:
I suppose you'll need two tuplestores for the ON CONFLICT DO UPDATE
case -- one for updated tuples, and the other for inserted tuples.[...]
Here is a patch implementing the above. It should be applied on top
of transition-tuples-from-wctes-v2.patch[2].
Here's a new version of patch #3. It's rebased on top of
transition-tuples-from-wctes-v3.patch. I also moved a comment for
execReplication.c out of this patch into patch #2, correcting a
mistake in my pancake stacking.
--
Thomas Munro
http://www.enterprisedb.com
Attachments:
transition-tuples-from-on-conflict-v2.patchapplication/octet-stream; name=transition-tuples-from-on-conflict-v2.patchDownload
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index e0c7c96e63a..23de6bfb896 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) ||
@@ -2128,8 +2145,10 @@ MakeTransitionCaptureState(TriggerDesc *trigdesc)
CurrentResourceOwner = CurTransactionResourceOwner;
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();
{
@@ -2147,8 +2166,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);
@@ -3994,7 +4015,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;
+ }
}
/*
@@ -5242,7 +5285,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 24586439de9..0a98fbfdf54 100644
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -406,12 +406,6 @@ 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 = <NULL>, new table = <NULL>
+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 = <NULL>
+--
+-- 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();
Re: Transition tables vs ON CONFLICT
On Mon, Jun 12, 2017 at 2:40 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
On Fri, Jun 9, 2017 at 4:10 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:On Wed, Jun 7, 2017 at 10:47 AM, Peter Geoghegan <pg@bowt.ie> wrote:
I suppose you'll need two tuplestores for the ON CONFLICT DO UPDATE
case -- one for updated tuples, and the other for inserted tuples.[...]
Here is a patch implementing the above. It should be applied on top
of transition-tuples-from-wctes-v2.patch[2].Here's a new version of patch #3.
That accidentally removed a comment that I wanted to keep. Here is a
better version.
--
Thomas Munro
http://www.enterprisedb.com
Attachments:
transition-tuples-from-on-conflict-v3.patchapplication/octet-stream; name=transition-tuples-from-on-conflict-v3.patchDownload
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index e0c7c96e63a..23de6bfb896 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) ||
@@ -2128,8 +2145,10 @@ MakeTransitionCaptureState(TriggerDesc *trigdesc)
CurrentResourceOwner = CurTransactionResourceOwner;
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();
{
@@ -2147,8 +2166,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);
@@ -3994,7 +4015,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;
+ }
}
/*
@@ -5242,7 +5285,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/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 = <NULL>, new table = <NULL>
+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 = <NULL>
+--
+-- 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();
Re: Transition tables vs ON CONFLICT
"Thomas" == Thomas Munro <thomas.munro@enterprisedb.com> writes:
Thomas> That accidentally removed a comment that I wanted to keep.
Thomas> Here is a better version.
I plan to commit this soon; if anyone has any comment to make, now would
be a good time.
--
Andrew (irc:RhodiumToad)
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: Transition tables vs ON CONFLICT
On Mon, Jun 19, 2017 at 11:04 AM, Andrew Gierth
<andrew@tao11.riddles.org.uk> wrote:
"Thomas" == Thomas Munro <thomas.munro@enterprisedb.com> writes:
Thomas> That accidentally removed a comment that I wanted to keep.
Thomas> Here is a better version.I plan to commit this soon; if anyone has any comment to make, now would
be a good time.
Here's patch #3 rebased for the recent reindent.
--
Thomas Munro
http://www.enterprisedb.com
Attachments:
transition-tuples-from-on-conflict-v4.patchapplication/octet-stream; name=transition-tuples-from-on-conflict-v4.patchDownload
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 54db16c9090..b502941b08b 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) ||
@@ -2128,8 +2145,10 @@ MakeTransitionCaptureState(TriggerDesc *trigdesc)
CurrentResourceOwner = CurTransactionResourceOwner;
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();
{
@@ -2147,8 +2166,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);
@@ -3993,7 +4014,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;
+ }
}
/*
@@ -5241,7 +5284,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/include/commands/trigger.h b/src/include/commands/trigger.h
index 06199953fe9..36c1134b649 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 35b83f7b009..71099969a47 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 = <NULL>, new table = <NULL>
+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 = <NULL>
+--
+-- 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 f957d70c5ff..771d68282ee 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();