v3 fixes a brown-paper-bag logic error. -- Justin
>From b5de1fc71f805bb8c7ec7e77bfce9a604ccd4bc8 Mon Sep 17 00:00:00 2001 From: Justin Pryzby <pryz...@telsasoft.com> Date: Fri, 3 Apr 2020 22:43:26 -0500 Subject: [PATCH v3] fix detaching tables with inherited row triggers
The old behavior is buggy, and the intended behavior was not previously documented. So define the behavior that the trigger is removed if the table is detached. It is an error if a table being attached already has a trigger of the same. This differs from the behavior for indexes and constraints. See also: 86f575948c773b0ec5b0f27066e37dd93a7f0a96 Discussion: https://www.postgresql.org/message-id/flat/20200408152412.GZ2228%40telsasoft.com --- doc/src/sgml/ref/create_trigger.sgml | 1 + src/backend/commands/tablecmds.c | 38 ++++++++++++++++++++++ src/bin/psql/describe.c | 12 +++++-- src/test/regress/expected/triggers.out | 44 ++++++++++++++++++++++++++ src/test/regress/sql/triggers.sql | 18 +++++++++++ 5 files changed, 111 insertions(+), 2 deletions(-) diff --git a/doc/src/sgml/ref/create_trigger.sgml b/doc/src/sgml/ref/create_trigger.sgml index bde3a63f90..303881fcfb 100644 --- a/doc/src/sgml/ref/create_trigger.sgml +++ b/doc/src/sgml/ref/create_trigger.sgml @@ -526,6 +526,7 @@ UPDATE OF <replaceable>column_name1</replaceable> [, <replaceable>column_name2</ Creating a row-level trigger on a partitioned table will cause identical triggers to be created in all its existing partitions; and any partitions created or attached later will contain an identical trigger, too. + If the partition is detached from its parent, the trigger is removed. Triggers on partitioned tables may not be <literal>INSTEAD OF</literal>. </para> diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 037d457c3d..480ea777ac 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -16797,6 +16797,44 @@ ATExecDetachPartition(Relation rel, RangeVar *name) } table_close(classRel, RowExclusiveLock); + /* + * Drop any ROW triggers inherited from partitioned table. + * This undoes what CloneRowTriggersToPartition did. + */ + { + ScanKeyData skey; + SysScanDesc scan; + HeapTuple trigtup; + Relation tgrel; + + ScanKeyInit(&skey, Anum_pg_trigger_tgrelid, BTEqualStrategyNumber, + F_OIDEQ, ObjectIdGetDatum(RelationGetRelid(partRel))); + tgrel = table_open(TriggerRelationId, RowExclusiveLock); + scan = systable_beginscan(tgrel, TriggerRelidNameIndexId, + true, NULL, 1, &skey); + + while (HeapTupleIsValid(trigtup = systable_getnext(scan))) + { + Form_pg_trigger pg_trigger = (Form_pg_trigger) GETSTRUCT(trigtup); + + if (!OidIsValid(pg_trigger->tgparentid) || + !pg_trigger->tgisinternal || + !TRIGGER_FOR_ROW(pg_trigger->tgtype)) + continue; + + RemoveTriggerById(pg_trigger->oid); + deleteDependencyRecordsFor(TriggerRelationId, + pg_trigger->oid, + false); + deleteDependencyRecordsFor(RelationRelationId, + pg_trigger->oid, + false); + } + + systable_endscan(scan); + table_close(tgrel, RowExclusiveLock); + } + /* * Detach any foreign keys that are inherited. This includes creating * additional action triggers. diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c index f05e914b4d..4cbc741c97 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -2939,14 +2939,17 @@ describeOneTableDetails(const char *schemaname, printfPQExpBuffer(&buf, "SELECT t.tgname, " "pg_catalog.pg_get_triggerdef(t.oid%s), " - "t.tgenabled, %s\n" + "t.tgenabled, %s, %s\n" "FROM pg_catalog.pg_trigger t\n" "WHERE t.tgrelid = '%s' AND ", (pset.sversion >= 90000 ? ", true" : ""), (pset.sversion >= 90000 ? "t.tgisinternal" : pset.sversion >= 80300 ? "t.tgconstraint <> 0 AS tgisinternal" : - "false AS tgisinternal"), oid); + "false AS tgisinternal"), + (pset.sversion >= 13000 ? "t.tgparentid" : + "0 AS tgparentid"), + oid); if (pset.sversion >= 110000) appendPQExpBufferStr(&buf, "(NOT t.tgisinternal OR (t.tgisinternal AND t.tgenabled = 'D') \n" " OR EXISTS (SELECT 1 FROM pg_catalog.pg_depend WHERE objid = t.oid \n" @@ -3062,6 +3065,11 @@ describeOneTableDetails(const char *schemaname, tgdef = usingpos + 9; printfPQExpBuffer(&buf, " %s", tgdef); + + /* Visually distinguish inherited triggers XXX: ROW only? */ + if (*PQgetvalue(result, i, 4) != '0') + appendPQExpBufferStr(&buf, ", PARTITION"); + printTableAddFooter(&cont, buf.data); } } diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out index e9da4ef983..9b544148bf 100644 --- a/src/test/regress/expected/triggers.out +++ b/src/test/regress/expected/triggers.out @@ -2023,6 +2023,50 @@ select tgrelid::regclass, tgname, tgfoid::regproc from pg_trigger ---------+--------+-------- (0 rows) +-- check detach behavior +create trigger trg1 after insert on trigpart for each row execute procedure trigger_nothing(); +\d trigpart3 + Table "public.trigpart3" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | + b | integer | | | +Partition of: trigpart FOR VALUES FROM (2000) TO (3000) +Triggers: + trg1 AFTER INSERT ON trigpart3 FOR EACH ROW EXECUTE FUNCTION trigger_nothing(), PARTITION + +alter table trigpart detach partition trigpart3; +drop trigger trg1 on trigpart3; -- fail due to "does not exist" +ERROR: trigger "trg1" for table "trigpart3" does not exist +alter table trigpart attach partition trigpart3 for values from (2000)to(3000); +alter table trigpart detach partition trigpart3; +alter table trigpart attach partition trigpart3 for values from (2000)to(3000); +drop table trigpart3; +select tgrelid::regclass::text, tgname, tgfoid::regproc, tgenabled, tgisinternal from pg_trigger + where tgname~'^trg1' order by 1; + tgrelid | tgname | tgfoid | tgenabled | tgisinternal +------------+--------+-----------------+-----------+-------------- + trigpart | trg1 | trigger_nothing | O | f + trigpart1 | trg1 | trigger_nothing | O | t + trigpart4 | trg1 | trigger_nothing | O | t + trigpart41 | trg1 | trigger_nothing | O | t + trigpart42 | trg1 | trigger_nothing | O | t +(5 rows) + +create table trigpart3 (like trigpart); +create trigger trg1 after insert on trigpart3 for each row execute procedure trigger_nothing(); +\d trigpart3 + Table "public.trigpart3" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | + b | integer | | | +Triggers: + trg1 AFTER INSERT ON trigpart3 FOR EACH ROW EXECUTE FUNCTION trigger_nothing() + +alter table trigpart attach partition trigpart3 FOR VALUES FROM (2000)to(3000); -- fail +ERROR: trigger "trg1" for relation "trigpart3" already exists +drop table trigpart3; drop table trigpart; drop function trigger_nothing(); -- diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql index 80ffbb4b02..5ab7cc62d2 100644 --- a/src/test/regress/sql/triggers.sql +++ b/src/test/regress/sql/triggers.sql @@ -1380,6 +1380,24 @@ drop trigger trg1 on trigpart; -- ok, all gone select tgrelid::regclass, tgname, tgfoid::regproc from pg_trigger where tgrelid::regclass::text like 'trigpart%' order by tgrelid::regclass::text; +-- check detach behavior +create trigger trg1 after insert on trigpart for each row execute procedure trigger_nothing(); +\d trigpart3 +alter table trigpart detach partition trigpart3; +drop trigger trg1 on trigpart3; -- fail due to "does not exist" +alter table trigpart attach partition trigpart3 for values from (2000)to(3000); +alter table trigpart detach partition trigpart3; +alter table trigpart attach partition trigpart3 for values from (2000)to(3000); +drop table trigpart3; + +select tgrelid::regclass::text, tgname, tgfoid::regproc, tgenabled, tgisinternal from pg_trigger + where tgname~'^trg1' order by 1; +create table trigpart3 (like trigpart); +create trigger trg1 after insert on trigpart3 for each row execute procedure trigger_nothing(); +\d trigpart3 +alter table trigpart attach partition trigpart3 FOR VALUES FROM (2000)to(3000); -- fail +drop table trigpart3; + drop table trigpart; drop function trigger_nothing(); -- 2.17.0
