Skip to content

Commit f61e601

Browse files
tglsfdctglsfdc
committed
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
1 parent 9d41ecf commit f61e601

File tree

6 files changed

+98
-20
lines changed

6 files changed

+98
-20
lines changed

‎doc/src/sgml/ref/alter_table.sgml

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -572,12 +572,12 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
572572
<para>
573573
These forms configure the firing of trigger(s) belonging to the table.
574574
A disabled trigger is still known to the system, but is not executed
575-
when its triggering event occurs. For a deferred trigger, the enable
575+
when its triggering event occurs. (For a deferred trigger, the enable
576576
status is checked when the event occurs, not when the trigger function
577-
is actually executed. One can disable or enable a single
577+
is actually executed.) One can disable or enable a single
578578
trigger specified by name, or all triggers on the table, or only
579579
user triggers (this option excludes internally generated constraint
580-
triggers such as those that are used to implement foreign key
580+
triggers, such as those that are used to implement foreign key
581581
constraints or deferrable uniqueness and exclusion constraints).
582582
Disabling or enabling internally generated constraint triggers
583583
requires superuser privileges; it should be done with caution since
@@ -599,14 +599,20 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
599599
The effect of this mechanism is that in the default configuration,
600600
triggers do not fire on replicas. This is useful because if a trigger
601601
is used on the origin to propagate data between tables, then the
602-
replication system will also replicate the propagated data, and the
602+
replication system will also replicate the propagated data; so the
603603
trigger should not fire a second time on the replica, because that would
604604
lead to duplication. However, if a trigger is used for another purpose
605605
such as creating external alerts, then it might be appropriate to set it
606606
to <literal>ENABLE ALWAYS</literal> so that it is also fired on
607607
replicas.
608608
</para>
609609

610+
<para>
611+
When this command is applied to a partitioned table, the states of
612+
corresponding clone triggers in the partitions are updated too,
613+
unless <literal>ONLY</literal> is specified.
614+
</para>
615+
610616
<para>
611617
This command acquires a <literal>SHARE ROW EXCLUSIVE</literal> lock.
612618
</para>
@@ -1234,7 +1240,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
12341240
<para>
12351241
Disable or enable all triggers belonging to the table.
12361242
(This requires superuser privilege if any of the triggers are
1237-
internally generated constraint triggers such as those that are used
1243+
internally generated constraint triggers, such as those that are used
12381244
to implement foreign key constraints or deferrable uniqueness and
12391245
exclusion constraints.)
12401246
</para>
@@ -1246,7 +1252,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
12461252
<listitem>
12471253
<para>
12481254
Disable or enable all triggers belonging to the table except for
1249-
internally generated constraint triggers such as those that are used
1255+
internally generated constraint triggers, such as those that are used
12501256
to implement foreign key constraints or deferrable uniqueness and
12511257
exclusion constraints.
12521258
</para>
@@ -1499,9 +1505,12 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
14991505
The actions for identity columns (<literal>ADD
15001506
GENERATED</literal>, <literal>SET</literal> etc., <literal>DROP
15011507
IDENTITY</literal>), as well as the actions
1502-
<literal>TRIGGER</literal>, <literal>CLUSTER</literal>, <literal>OWNER</literal>,
1508+
<literal>CLUSTER</literal>, <literal>OWNER</literal>,
15031509
and <literal>TABLESPACE</literal> never recurse to descendant tables;
15041510
that is, they always act as though <literal>ONLY</literal> were specified.
1511+
Actions affecting trigger states recurse to partitions of partitioned
1512+
tables (unless <literal>ONLY</literal> is specified), but never to
1513+
traditional-inheritance descendants.
15051514
Adding a constraint recurses only for <literal>CHECK</literal> constraints
15061515
that are not marked <literal>NO INHERIT</literal>.
15071516
</para>

‎src/backend/commands/tablecmds.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14768,8 +14768,9 @@ ATExecEnableDisableTrigger(Relation rel, const char *trigname,
1476814768
char fires_when, bool skip_system, bool recurse,
1476914769
LOCKMODE lockmode)
1477014770
{
14771-
EnableDisableTriggerNew(rel, trigname, fires_when, skip_system, recurse,
14772-
lockmode);
14771+
EnableDisableTriggerNew2(rel, trigname, InvalidOid,
14772+
fires_when, skip_system, recurse,
14773+
lockmode);
1477314774
}
1477414775

1477514776
/*

‎src/backend/commands/trigger.c

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1754,7 +1754,8 @@ renametrig_partition(Relation tgrel, Oid partitionId, Oid parentTriggerOid,
17541754
* to change 'tgenabled' field for the specified trigger(s)
17551755
*
17561756
* rel: relation to process (caller must hold suitable lock on it)
1757-
* tgname: trigger to process, or NULL to scan all triggers
1757+
* tgname: name of trigger to process, or NULL to scan all triggers
1758+
* tgparent: if not zero, process only triggers with this tgparentid
17581759
* fires_when: new value for tgenabled field. In addition to generic
17591760
* enablement/disablement, this also defines when the trigger
17601761
* should be fired in session replication roles.
@@ -1766,9 +1767,9 @@ renametrig_partition(Relation tgrel, Oid partitionId, Oid parentTriggerOid,
17661767
* system triggers
17671768
*/
17681769
void
1769-
EnableDisableTriggerNew(Relation rel, const char *tgname,
1770-
char fires_when, bool skip_system, bool recurse,
1771-
LOCKMODE lockmode)
1770+
EnableDisableTriggerNew2(Relation rel, const char *tgname, Oid tgparent,
1771+
char fires_when, bool skip_system, bool recurse,
1772+
LOCKMODE lockmode)
17721773
{
17731774
Relation tgrel;
17741775
int nkeys;
@@ -1805,6 +1806,9 @@ EnableDisableTriggerNew(Relation rel, const char *tgname,
18051806
{
18061807
Form_pg_trigger oldtrig = (Form_pg_trigger) GETSTRUCT(tuple);
18071808

1809+
if (OidIsValid(tgparent) && tgparent != oldtrig->tgparentid)
1810+
continue;
1811+
18081812
if (oldtrig->tgisinternal)
18091813
{
18101814
/* system trigger ... ok to process? */
@@ -1855,9 +1859,10 @@ EnableDisableTriggerNew(Relation rel, const char *tgname,
18551859
Relation part;
18561860

18571861
part = relation_open(partdesc->oids[i], lockmode);
1858-
EnableDisableTriggerNew(part, NameStr(oldtrig->tgname),
1859-
fires_when, skip_system, recurse,
1860-
lockmode);
1862+
/* Match on child triggers' tgparentid, not their name */
1863+
EnableDisableTriggerNew2(part, NULL, oldtrig->oid,
1864+
fires_when, skip_system, recurse,
1865+
lockmode);
18611866
table_close(part, NoLock); /* keep lock till commit */
18621867
}
18631868
}
@@ -1886,16 +1891,27 @@ EnableDisableTriggerNew(Relation rel, const char *tgname,
18861891
}
18871892

18881893
/*
1889-
* ABI-compatible wrapper for the above. To keep as close possible to the old
1890-
* behavior, this never recurses. Do not call this function in new code.
1894+
* ABI-compatible wrappers to emulate old versions of the above function.
1895+
* Do not call these versions in new code.
18911896
*/
1897+
void
1898+
EnableDisableTriggerNew(Relation rel, const char *tgname,
1899+
char fires_when, bool skip_system, bool recurse,
1900+
LOCKMODE lockmode)
1901+
{
1902+
EnableDisableTriggerNew2(rel, tgname, InvalidOid,
1903+
fires_when, skip_system,
1904+
recurse, lockmode);
1905+
}
1906+
18921907
void
18931908
EnableDisableTrigger(Relation rel, const char *tgname,
18941909
char fires_when, bool skip_system,
18951910
LOCKMODE lockmode)
18961911
{
1897-
EnableDisableTriggerNew(rel, tgname, fires_when, skip_system,
1898-
true, lockmode);
1912+
EnableDisableTriggerNew2(rel, tgname, InvalidOid,
1913+
fires_when, skip_system,
1914+
true, lockmode);
18991915
}
19001916

19011917

‎src/include/commands/trigger.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,9 @@ extern Oid get_trigger_oid(Oid relid, const char *name, bool missing_ok);
170170

171171
extern ObjectAddress renametrig(RenameStmt *stmt);
172172

173+
extern void EnableDisableTriggerNew2(Relation rel, const char *tgname, Oid tgparent,
174+
char fires_when, bool skip_system, bool recurse,
175+
LOCKMODE lockmode);
173176
extern void EnableDisableTriggerNew(Relation rel, const char *tgname,
174177
char fires_when, bool skip_system, bool recurse,
175178
LOCKMODE lockmode);

‎src/test/regress/expected/triggers.out

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2718,6 +2718,40 @@ select tgrelid::regclass, tgname, tgenabled from pg_trigger
27182718
parent | tg_stmt | A
27192719
(3 rows)
27202720

2721+
drop table parent, child1;
2722+
-- Check processing of foreign key triggers
2723+
create table parent (a int primary key, f int references parent)
2724+
partition by list (a);
2725+
create table child1 partition of parent for values in (1);
2726+
select tgrelid::regclass, rtrim(tgname, '0123456789') as tgname,
2727+
tgfoid::regproc, tgenabled
2728+
from pg_trigger where tgrelid in ('parent'::regclass, 'child1'::regclass)
2729+
order by tgrelid::regclass::text, tgfoid;
2730+
tgrelid | tgname | tgfoid | tgenabled
2731+
---------+-------------------------+------------------------+-----------
2732+
child1 | RI_ConstraintTrigger_c_ | "RI_FKey_check_ins" | O
2733+
child1 | RI_ConstraintTrigger_c_ | "RI_FKey_check_upd" | O
2734+
parent | RI_ConstraintTrigger_c_ | "RI_FKey_check_ins" | O
2735+
parent | RI_ConstraintTrigger_c_ | "RI_FKey_check_upd" | O
2736+
parent | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_del" | O
2737+
parent | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_upd" | O
2738+
(6 rows)
2739+
2740+
alter table parent disable trigger all;
2741+
select tgrelid::regclass, rtrim(tgname, '0123456789') as tgname,
2742+
tgfoid::regproc, tgenabled
2743+
from pg_trigger where tgrelid in ('parent'::regclass, 'child1'::regclass)
2744+
order by tgrelid::regclass::text, tgfoid;
2745+
tgrelid | tgname | tgfoid | tgenabled
2746+
---------+-------------------------+------------------------+-----------
2747+
child1 | RI_ConstraintTrigger_c_ | "RI_FKey_check_ins" | D
2748+
child1 | RI_ConstraintTrigger_c_ | "RI_FKey_check_upd" | D
2749+
parent | RI_ConstraintTrigger_c_ | "RI_FKey_check_ins" | D
2750+
parent | RI_ConstraintTrigger_c_ | "RI_FKey_check_upd" | D
2751+
parent | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_del" | D
2752+
parent | RI_ConstraintTrigger_a_ | "RI_FKey_noaction_upd" | D
2753+
(6 rows)
2754+
27212755
drop table parent, child1;
27222756
-- Verify that firing state propagates correctly on creation, too
27232757
CREATE TABLE trgfire (i int) PARTITION BY RANGE (i);

‎src/test/regress/sql/triggers.sql

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1883,6 +1883,21 @@ select tgrelid::regclass, tgname, tgenabled from pg_trigger
18831883
order by tgrelid::regclass::text, tgname;
18841884
drop table parent, child1;
18851885

1886+
-- Check processing of foreign key triggers
1887+
create table parent (a int primary key, f int references parent)
1888+
partition by list (a);
1889+
create table child1 partition of parent for values in (1);
1890+
select tgrelid::regclass, rtrim(tgname, '0123456789') as tgname,
1891+
tgfoid::regproc, tgenabled
1892+
from pg_trigger where tgrelid in ('parent'::regclass, 'child1'::regclass)
1893+
order by tgrelid::regclass::text, tgfoid;
1894+
alter table parent disable trigger all;
1895+
select tgrelid::regclass, rtrim(tgname, '0123456789') as tgname,
1896+
tgfoid::regproc, tgenabled
1897+
from pg_trigger where tgrelid in ('parent'::regclass, 'child1'::regclass)
1898+
order by tgrelid::regclass::text, tgfoid;
1899+
drop table parent, child1;
1900+
18861901
-- Verify that firing state propagates correctly on creation, too
18871902
CREATE TABLE trgfire (i int) PARTITION BY RANGE (i);
18881903
CREATE TABLE trgfire1 PARTITION OF trgfire FOR VALUES FROM (1) TO (10);

0 commit comments

Comments
 (0)