postgres.git / summary / log / commit / refs
commit 6949b921d545809a83f8a6bad4948f9012a76fb6
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sat Mar 04 18:32:35 2023 +0000
First released in: 16.0, 15.3
Avoid failure when altering state of partitioned foreign-key triggers.
Beginning in v15, if you apply ALTER TABLE ENABLE/DISABLE TRIGGER to
a partitioned table, it also affects the partitions' cloned versions
of the affected trigger(s). The initial implementation of this
located the clones by name, but that fails on foreign-key triggers
which have names incorporating their own OIDs. We can fix that, and
also make the behavior more bulletproof in the face of user-initiated
trigger renames, by identifying the cloned triggers by tgparentid.
Following the lead of earlier commits in this area, I took care not
to break ABI in the v15 branch, even though I rather doubt there
are any external callers of EnableDisableTrigger.
While here, update the documentation, which was not touched when
the semantics were changed.
Per bug #17817 from Alan Hodgson. Back-patch to v15; older versions
do not have this behavior.
Discussion: https://postgr.es/m/17817-31dfb7c2100d9f3d@postgresql.org
doc/src/sgml/ref/alter_table.sgml | 23 ++++++++++++++++-------
src/backend/commands/tablecmds.c | 3 ++-
src/backend/commands/trigger.c | 11 ++++++++---
src/include/commands/trigger.h | 2 +-
src/test/regress/expected/triggers.out | 34 ++++++++++++++++++++++++++++++++++
src/test/regress/sql/triggers.sql | 15 +++++++++++++++
6 files changed, 76 insertions(+), 12 deletions(-)
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 9aaa32a782c..d4d93eeb7c6 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -576,12 +576,12 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
<para>
These forms configure the firing of trigger(s) belonging to the table.
A disabled trigger is still known to the system, but is not executed
- when its triggering event occurs. For a deferred trigger, the enable
+ when its triggering event occurs. (For a deferred trigger, the enable
status is checked when the event occurs, not when the trigger function
- is actually executed. One can disable or enable a single
+ is actually executed.) One can disable or enable a single
trigger specified by name, or all triggers on the table, or only
user triggers (this option excludes internally generated constraint
- triggers such as those that are used to implement foreign key
+ triggers, such as those that are used to implement foreign key
constraints or deferrable uniqueness and exclusion constraints).
Disabling or enabling internally generated constraint triggers
requires superuser privileges; it should be done with caution since
@@ -603,7 +603,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
The effect of this mechanism is that in the default configuration,
triggers do not fire on replicas. This is useful because if a trigger
is used on the origin to propagate data between tables, then the
- replication system will also replicate the propagated data, and the
+ replication system will also replicate the propagated data; so the
trigger should not fire a second time on the replica, because that would
lead to duplication. However, if a trigger is used for another purpose
such as creating external alerts, then it might be appropriate to set it
@@ -611,6 +611,12 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
replicas.
</para>
+ <para>
+ When this command is applied to a partitioned table, the states of
+ corresponding clone triggers in the partitions are updated too,
+ unless <literal>ONLY</literal> is specified.
+ </para>
+
<para>
This command acquires a <literal>SHARE ROW EXCLUSIVE</literal> lock.
</para>
@@ -1239,7 +1245,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
<para>
Disable or enable all triggers belonging to the table.
(This requires superuser privilege if any of the triggers are
- internally generated constraint triggers such as those that are used
+ internally generated constraint triggers, such as those that are used
to implement foreign key constraints or deferrable uniqueness and
exclusion constraints.)
</para>
@@ -1251,7 +1257,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
<listitem>
<para>
Disable or enable all triggers belonging to the table except for
- internally generated constraint triggers such as those that are used
+ internally generated constraint triggers, such as those that are used
to implement foreign key constraints or deferrable uniqueness and
exclusion constraints.
</para>
@@ -1504,9 +1510,12 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
The actions for identity columns (<literal>ADD
GENERATED</literal>, <literal>SET</literal> etc., <literal>DROP
IDENTITY</literal>), as well as the actions
- <literal>TRIGGER</literal>, <literal>CLUSTER</literal>, <literal>OWNER</literal>,
+ <literal>CLUSTER</literal>, <literal>OWNER</literal>,
and <literal>TABLESPACE</literal> never recurse to descendant tables;
that is, they always act as though <literal>ONLY</literal> were specified.
+ Actions affecting trigger states recurse to partitions of partitioned
+ tables (unless <literal>ONLY</literal> is specified), but never to
+ traditional-inheritance descendants.
Adding a constraint recurses only for <literal>CHECK</literal> constraints
that are not marked <literal>NO INHERIT</literal>.
</para>
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 62d9917ca36..3a93c41d6ab 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -14726,7 +14726,8 @@ ATExecEnableDisableTrigger(Relation rel, const char *trigname,
char fires_when, bool skip_system, bool recurse,
LOCKMODE lockmode)
{
- EnableDisableTrigger(rel, trigname, fires_when, skip_system, recurse,
+ EnableDisableTrigger(rel, trigname, InvalidOid,
+ fires_when, skip_system, recurse,
lockmode);
}
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index d7abde170be..66401f28392 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -1715,7 +1715,8 @@ renametrig_partition(Relation tgrel, Oid partitionId, Oid parentTriggerOid,
* to change 'tgenabled' field for the specified trigger(s)
*
* rel: relation to process (caller must hold suitable lock on it)
- * tgname: trigger to process, or NULL to scan all triggers
+ * tgname: name of trigger to process, or NULL to scan all triggers
+ * tgparent: if not zero, process only triggers with this tgparentid
* fires_when: new value for tgenabled field. In addition to generic
* enablement/disablement, this also defines when the trigger
* should be fired in session replication roles.
@@ -1727,7 +1728,7 @@ renametrig_partition(Relation tgrel, Oid partitionId, Oid parentTriggerOid,
* system triggers
*/
void
-EnableDisableTrigger(Relation rel, const char *tgname,
+EnableDisableTrigger(Relation rel, const char *tgname, Oid tgparent,
char fires_when, bool skip_system, bool recurse,
LOCKMODE lockmode)
{
@@ -1766,6 +1767,9 @@ EnableDisableTrigger(Relation rel, const char *tgname,
{
Form_pg_trigger oldtrig = (Form_pg_trigger) GETSTRUCT(tuple);
+ if (OidIsValid(tgparent) && tgparent != oldtrig->tgparentid)
+ continue;
+
if (oldtrig->tgisinternal)
{
/* system trigger ... ok to process? */
@@ -1816,7 +1820,8 @@ EnableDisableTrigger(Relation rel, const char *tgname,
Relation part;
part = relation_open(partdesc->oids[i], lockmode);
- EnableDisableTrigger(part, NameStr(oldtrig->tgname),
+ /* Match on child triggers' tgparentid, not their name */
+ EnableDisableTrigger(part, NULL, oldtrig->oid,
fires_when, skip_system, recurse,
lockmode);
table_close(part, NoLock); /* keep lock till commit */
diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h
index 39d764dac70..67496d84f98 100644
--- a/src/include/commands/trigger.h
+++ b/src/include/commands/trigger.h
@@ -170,7 +170,7 @@ extern Oid get_trigger_oid(Oid relid, const char *trigname, bool missing_ok);
extern ObjectAddress renametrig(RenameStmt *stmt);
-extern void EnableDisableTrigger(Relation rel, const char *tgname,
+extern void EnableDisableTrigger(Relation rel, const char *tgname, Oid tgparent,
char fires_when, bool skip_system, bool recurse,
LOCKMODE lockmode);
diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out
index 7dbeced570d..f7301b52130 100644
--- a/src/test/regress/expected/triggers.out
+++ b/src/test/regress/expected/triggers.out
@@ -2718,6 +2718,40 @@ select tgrelid::regclass, tgname, tgenabled from pg_trigger
parent | tg_stmt | A
(3 rows)
+drop table parent, child1;
+-- Check processing of foreign key triggers
+create table parent (a int primary key, f int references parent)
+ partition by list (a);
+create table child1 partition of parent for values in (1);
+select tgrelid::regclass, rtrim(tgname, '0123456789') as tgname,
+ tgfoid::regproc, tgenabled
+ from pg_trigger where tgrelid in ('parent'::regclass, 'child1'::regclass)
+ order by tgrelid::regclass::text, tgfoid;
+ tgrelid | tgname | tgfoid | tgenabled
+---------+-------------------------+------------------------+-----------
+ child1 | RI_ConstraintTrigger_c_ | "RI_FKey_check_ins" | O
+ child1 | RI_ConstraintTrigger_c_ | "RI_FKey_check_upd" | O
+ parent | RI_ConstraintTrigger_c_ | "RI_FKey_check_ins" | O
+ parent | RI_ConstraintTrigger_c_ | "RI_FKey_check_upd" | O
+ parent | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_del" | O
+ parent | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_upd" | O
+(6 rows)
+
+alter table parent disable trigger all;
+select tgrelid::regclass, rtrim(tgname, '0123456789') as tgname,
+ tgfoid::regproc, tgenabled
+ from pg_trigger where tgrelid in ('parent'::regclass, 'child1'::regclass)
+ order by tgrelid::regclass::text, tgfoid;
+ tgrelid | tgname | tgfoid | tgenabled
+---------+-------------------------+------------------------+-----------
+ child1 | RI_ConstraintTrigger_c_ | "RI_FKey_check_ins" | D
+ child1 | RI_ConstraintTrigger_c_ | "RI_FKey_check_upd" | D
+ parent | RI_ConstraintTrigger_c_ | "RI_FKey_check_ins" | D
+ parent | RI_ConstraintTrigger_c_ | "RI_FKey_check_upd" | D
+ parent | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_del" | D
+ parent | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_upd" | D
+(6 rows)
+
drop table parent, child1;
-- Verify that firing state propagates correctly on creation, too
CREATE TABLE trgfire (i int) PARTITION BY RANGE (i);
diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql
index 4d8504fb246..4222a8500b4 100644
--- a/src/test/regress/sql/triggers.sql
+++ b/src/test/regress/sql/triggers.sql
@@ -1883,6 +1883,21 @@ select tgrelid::regclass, tgname, tgenabled from pg_trigger
order by tgrelid::regclass::text, tgname;
drop table parent, child1;
+-- Check processing of foreign key triggers
+create table parent (a int primary key, f int references parent)
+ partition by list (a);
+create table child1 partition of parent for values in (1);
+select tgrelid::regclass, rtrim(tgname, '0123456789') as tgname,
+ tgfoid::regproc, tgenabled
+ from pg_trigger where tgrelid in ('parent'::regclass, 'child1'::regclass)
+ order by tgrelid::regclass::text, tgfoid;
+alter table parent disable trigger all;
+select tgrelid::regclass, rtrim(tgname, '0123456789') as tgname,
+ tgfoid::regproc, tgenabled
+ from pg_trigger where tgrelid in ('parent'::regclass, 'child1'::regclass)
+ order by tgrelid::regclass::text, tgfoid;
+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);
[parent: f62975b2af6a]