From 00708a471aabc3687862bf7151bea75c821c59ee Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Wed, 14 Jul 2021 16:27:40 -0400 Subject: [PATCH v4 1/2] Preserve firing-on state when cloning row triggers to partitions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When triggers are cloned from partitioned tables to their partitions, the 'tgenabled' flag (origin/replica/always/disable) was not propagated. Make it so that the flag on the trigger on partition is initially set to the same value as on the partitioned table. Add a test case to verify the behavior. Backpatch to 14. The original behavior, which appeared in pg11 with commit 86f575948c77 doesn't really make much sense, but it seems risky to change it on established branches. Author: Álvaro Herrera Reported-by: Justin Pryzby Discussion: https://postgr.es/m/20200930223450.GA14848@telsasoft.com --- src/backend/commands/tablecmds.c | 4 +- src/backend/commands/trigger.c | 30 +++++++++++--- src/include/commands/trigger.h | 5 +++ src/test/regress/expected/triggers.out | 56 ++++++++++++++++++++++++++ src/test/regress/sql/triggers.sql | 32 +++++++++++++++ 5 files changed, 119 insertions(+), 8 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 96375814a8..dd2aefe1f3 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -17736,10 +17736,10 @@ CloneRowTriggersToPartition(Relation parent, Relation partition) trigStmt->initdeferred = trigForm->tginitdeferred; trigStmt->constrrel = NULL; /* passed separately */ - CreateTrigger(trigStmt, NULL, RelationGetRelid(partition), + CreateTriggerFiringOn(trigStmt, NULL, RelationGetRelid(partition), trigForm->tgconstrrelid, InvalidOid, InvalidOid, trigForm->tgfoid, trigForm->oid, qual, - false, true); + false, true, trigForm->tgenabled); MemoryContextSwitchTo(oldcxt); MemoryContextReset(perTupCxt); diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 952c8d582a..6d4b7ee92a 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -151,6 +151,24 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, Oid relOid, Oid refRelOid, Oid constraintOid, Oid indexOid, Oid funcoid, Oid parentTriggerOid, Node *whenClause, bool isInternal, bool in_partition) +{ + return + CreateTriggerFiringOn(stmt, queryString, relOid, refRelOid, + constraintOid, indexOid, funcoid, + parentTriggerOid, whenClause, isInternal, + in_partition, TRIGGER_FIRES_ON_ORIGIN); +} + +/* + * Like the above; additionally the firing condition + * (always/origin/replica/disabled) can be specified. + */ +ObjectAddress +CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString, + Oid relOid, Oid refRelOid, Oid constraintOid, + Oid indexOid, Oid funcoid, Oid parentTriggerOid, + Node *whenClause, bool isInternal, bool in_partition, + char trigger_fires_when) { int16 tgtype; int ncolumns; @@ -849,7 +867,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, CStringGetDatum(trigname)); values[Anum_pg_trigger_tgfoid - 1] = ObjectIdGetDatum(funcoid); values[Anum_pg_trigger_tgtype - 1] = Int16GetDatum(tgtype); - values[Anum_pg_trigger_tgenabled - 1] = CharGetDatum(TRIGGER_FIRES_ON_ORIGIN); + values[Anum_pg_trigger_tgenabled - 1] = trigger_fires_when; values[Anum_pg_trigger_tgisinternal - 1] = BoolGetDatum(isInternal || in_partition); values[Anum_pg_trigger_tgconstrrelid - 1] = ObjectIdGetDatum(constrrelid); values[Anum_pg_trigger_tgconstrindid - 1] = ObjectIdGetDatum(indexOid); @@ -1196,11 +1214,11 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, map_partition_varattnos((List *) qual, PRS2_NEW_VARNO, childTbl, rel); - CreateTrigger(childStmt, queryString, - partdesc->oids[i], refRelOid, - InvalidOid, indexOnChild, - funcoid, trigoid, qual, - isInternal, true); + CreateTriggerFiringOn(childStmt, queryString, + partdesc->oids[i], refRelOid, + InvalidOid, indexOnChild, + funcoid, trigoid, qual, + isInternal, true, trigger_fires_when); table_close(childTbl, NoLock); diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h index 9e557cfbce..6474237a95 100644 --- a/src/include/commands/trigger.h +++ b/src/include/commands/trigger.h @@ -154,6 +154,11 @@ extern ObjectAddress CreateTrigger(CreateTrigStmt *stmt, const char *queryString Oid relOid, Oid refRelOid, Oid constraintOid, Oid indexOid, Oid funcoid, Oid parentTriggerOid, Node *whenClause, bool isInternal, bool in_partition); +extern ObjectAddress CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString, + Oid relOid, Oid refRelOid, Oid constraintOid, + Oid indexOid, Oid funcoid, Oid parentTriggerOid, + Node *whenClause, bool isInternal, bool in_partition, + char trigger_fires_when); extern void RemoveTriggerById(Oid trigOid); extern Oid get_trigger_oid(Oid relid, const char *name, bool missing_ok); diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out index e8af9a9589..42392f8f41 100644 --- a/src/test/regress/expected/triggers.out +++ b/src/test/regress/expected/triggers.out @@ -2661,6 +2661,62 @@ select tgrelid::regclass, tgname, tgenabled from pg_trigger (2 rows) drop table parent, child1; +-- Verify that firing state propagates correctly on creation, too +CREATE TABLE trgfire (i int) PARTITION BY RANGE (i); +CREATE TABLE trgfire1 PARTITION OF trgfire FOR VALUES FROM (1) TO (10); +CREATE OR REPLACE FUNCTION tgf() RETURNS trigger LANGUAGE plpgsql + AS $$ begin raise exception 'except'; end $$; +CREATE TRIGGER tg AFTER INSERT ON trgfire FOR EACH ROW EXECUTE FUNCTION tgf(); +INSERT INTO trgfire VALUES (1); +ERROR: except +CONTEXT: PL/pgSQL function tgf() line 1 at RAISE +ALTER TABLE trgfire DISABLE TRIGGER tg; +INSERT INTO trgfire VALUES (1); +CREATE TABLE trgfire2 PARTITION OF trgfire FOR VALUES FROM (10) TO (20); +INSERT INTO trgfire VALUES (11); +CREATE TABLE trgfire3 (LIKE trgfire); +ALTER TABLE trgfire ATTACH PARTITION trgfire3 FOR VALUES FROM (20) TO (30); +INSERT INTO trgfire VALUES (21); +CREATE TABLE trgfire4 PARTITION OF trgfire FOR VALUES FROM (30) TO (40) PARTITION BY LIST (i); +CREATE TABLE trgfire4_30 PARTITION OF trgfire4 FOR VALUES IN (30); +INSERT INTO trgfire VALUES (30); +CREATE TABLE trgfire5 (LIKE trgfire) PARTITION BY LIST (i); +CREATE TABLE trgfire5_40 PARTITION OF trgfire5 FOR VALUES IN (40); +ALTER TABLE trgfire ATTACH PARTITION trgfire5 FOR VALUES FROM (40) TO (50); +INSERT INTO trgfire VALUES (40); +SELECT tgrelid::regclass, tgenabled FROM pg_trigger + WHERE tgrelid::regclass IN (SELECT oid from pg_class where relname LIKE 'trgfire%') + ORDER BY tgrelid::regclass::text; + tgrelid | tgenabled +-------------+----------- + trgfire | D + trgfire1 | D + trgfire2 | D + trgfire3 | D + trgfire4 | D + trgfire4_30 | D + trgfire5 | D + trgfire5_40 | D +(8 rows) + +ALTER TABLE trgfire ENABLE TRIGGER tg; +INSERT INTO trgfire VALUES (1); +ERROR: except +CONTEXT: PL/pgSQL function tgf() line 1 at RAISE +INSERT INTO trgfire VALUES (11); +ERROR: except +CONTEXT: PL/pgSQL function tgf() line 1 at RAISE +INSERT INTO trgfire VALUES (21); +ERROR: except +CONTEXT: PL/pgSQL function tgf() line 1 at RAISE +INSERT INTO trgfire VALUES (30); +ERROR: except +CONTEXT: PL/pgSQL function tgf() line 1 at RAISE +INSERT INTO trgfire VALUES (40); +ERROR: except +CONTEXT: PL/pgSQL function tgf() line 1 at RAISE +DROP TABLE trgfire; +DROP FUNCTION tgf(); -- -- Test the interaction between transition tables and both kinds of -- inheritance. We'll dump the contents of the transition tables in a diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql index b50f500045..0777c4f50f 100644 --- a/src/test/regress/sql/triggers.sql +++ b/src/test/regress/sql/triggers.sql @@ -1836,6 +1836,38 @@ select tgrelid::regclass, tgname, tgenabled from pg_trigger order by tgrelid::regclass::text; drop table parent, child1; +-- Verify that firing state propagates correctly on creation, too +CREATE TABLE trgfire (i int) PARTITION BY RANGE (i); +CREATE TABLE trgfire1 PARTITION OF trgfire FOR VALUES FROM (1) TO (10); +CREATE OR REPLACE FUNCTION tgf() RETURNS trigger LANGUAGE plpgsql + AS $$ begin raise exception 'except'; end $$; +CREATE TRIGGER tg AFTER INSERT ON trgfire FOR EACH ROW EXECUTE FUNCTION tgf(); +INSERT INTO trgfire VALUES (1); +ALTER TABLE trgfire DISABLE TRIGGER tg; +INSERT INTO trgfire VALUES (1); +CREATE TABLE trgfire2 PARTITION OF trgfire FOR VALUES FROM (10) TO (20); +INSERT INTO trgfire VALUES (11); +CREATE TABLE trgfire3 (LIKE trgfire); +ALTER TABLE trgfire ATTACH PARTITION trgfire3 FOR VALUES FROM (20) TO (30); +INSERT INTO trgfire VALUES (21); +CREATE TABLE trgfire4 PARTITION OF trgfire FOR VALUES FROM (30) TO (40) PARTITION BY LIST (i); +CREATE TABLE trgfire4_30 PARTITION OF trgfire4 FOR VALUES IN (30); +INSERT INTO trgfire VALUES (30); +CREATE TABLE trgfire5 (LIKE trgfire) PARTITION BY LIST (i); +CREATE TABLE trgfire5_40 PARTITION OF trgfire5 FOR VALUES IN (40); +ALTER TABLE trgfire ATTACH PARTITION trgfire5 FOR VALUES FROM (40) TO (50); +INSERT INTO trgfire VALUES (40); +SELECT tgrelid::regclass, tgenabled FROM pg_trigger + WHERE tgrelid::regclass IN (SELECT oid from pg_class where relname LIKE 'trgfire%') + ORDER BY tgrelid::regclass::text; +ALTER TABLE trgfire ENABLE TRIGGER tg; +INSERT INTO trgfire VALUES (1); +INSERT INTO trgfire VALUES (11); +INSERT INTO trgfire VALUES (21); +INSERT INTO trgfire VALUES (30); +INSERT INTO trgfire VALUES (40); +DROP TABLE trgfire; +DROP FUNCTION tgf(); -- -- Test the interaction between transition tables and both kinds of -- 2.20.1