From: Tom Lane <tgl@sss.pgh.pa.us>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: ahodgson@simkin.ca
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17817: DISABLE TRIGGER ALL on a partitioned table with foreign key fails
Date: Fri, 03 Mar 2023 10:50:40 -0500
Message-ID: <2473213.1677858640@sss.pgh.pa.us> (raw)
In-Reply-To: <2416983.1677854831@sss.pgh.pa.us>
References: <20230303112144.tzw3yeos2c5w3q6y@alvherre.pgsql>
<2416983.1677854831@sss.pgh.pa.us>
I wrote:
> I can throw together a patch for what I was thinking of.
Basically just make the recursive steps match on tgparentid instead
of name, like this.
I wonder whether anyplace else is making a similar mistake? Although
there's not much we will let you do to a foreign key trigger, so it
might not matter for anything else.
regards, tom lane
Attachments:fix-bug-17817.patch (text/x-diff, 6.4K) downloaddiff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex 62d9917ca3..3a93c41d6a 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.cindex d7abde170b..66401f2839 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.hindex 39d764dac7..67496d84f9 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.outindex 7dbeced570..f7301b5213 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.sqlindex 4d8504fb24..4222a8500b 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);
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: tgl@sss.pgh.pa.us, alvherre@alvh.no-ip.org, ahodgson@simkin.ca, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17817: DISABLE TRIGGER ALL on a partitioned table with foreign key fails
In-Reply-To: <2473213.1677858640@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox