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
This commit is contained in:
Tom Lane 2023-03-04 13:32:35 -05:00
parent f62975b2af
commit 6949b921d5
6 changed files with 76 additions and 12 deletions

View File

@ -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>

View File

@ -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);
}

View File

@ -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 */

View File

@ -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);

View File

@ -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);

View File

@ -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);