From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: pgsql-hackers@postgresql.org, Amit Langote <amitlangote09@gmail.com>
Subject: Re: CREATE TABLE .. PARTITION OF fails to preserve tgenabled for inherited row triggers
Date: Fri, 16 Oct 2020 13:59:34 -0300
Message-ID: <20201016165934.GA5479@alvherre.pgsql> (raw)
In-Reply-To: <20201016135230.GA23633@alvherre.pgsql>
Same, with a little test.
I also just noticed that ALTER TABLE ONLY recurses to children, which it
should not.
Attachments:0001-When-cloning-triggers-preserve-enabling-state.patch (text/x-diff, 8.1K) download
From 2fb3a3122bdbbb1eb5aa6608b5132b8ab07096d4 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Fri, 16 Oct 2020 10:58:54 -0300
Subject: [PATCH] When cloning triggers, preserve enabling state
---
src/backend/commands/tablecmds.c | 8 +++---
src/backend/commands/trigger.c | 30 ++++++++++++++++----
src/include/commands/trigger.h | 5 ++++
src/test/regress/expected/triggers.out | 39 ++++++++++++++++++++++++++
src/test/regress/sql/triggers.sql | 24 ++++++++++++++++
5 files changed, 96 insertions(+), 10 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex 511f015a86..f20b877f68 100644--- a/src/backend/commands/tablecmds.c+++ b/src/backend/commands/tablecmds.c@@ -16879,10 +16879,10 @@ CloneRowTriggersToPartition(Relation parent, Relation partition)
trigStmt->initdeferred = trigForm->tginitdeferred;
trigStmt->constrrel = NULL; /* passed separately */
- CreateTrigger(trigStmt, NULL, RelationGetRelid(partition),- trigForm->tgconstrrelid, InvalidOid, InvalidOid,- trigForm->tgfoid, trigForm->oid, qual,- false, true);+ CreateTriggerFiringOn(trigStmt, NULL, RelationGetRelid(partition),+ trigForm->tgconstrrelid, InvalidOid, InvalidOid,+ trigForm->tgfoid, trigForm->oid, qual,+ false, true, trigForm->tgenabled);
MemoryContextSwitchTo(oldcxt);
MemoryContextReset(perTupCxt);
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.cindex 3b4fbdadf4..b826b3595c 100644--- a/src/backend/commands/trigger.c+++ b/src/backend/commands/trigger.c@@ -158,6 +158,24 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
Oid relOid, Oid refRelOid, Oid constraintOid, Oid indexOid,
Oid funcoid, Oid parentTriggerOid, Node *whenClause,
bool isInternal, bool in_partition)
+{+ return+ CreateTriggerFiringOn(stmt, queryString, relOid, refRelOid,+ constraintOid, indexOid, funcoid,+ parentTriggerOid, whenClause, isInternal,+ in_partition, TRIGGER_FIRES_ON_ORIGIN);+}++/*+ * Like the above; additionally the firing condition+ * (always/origin/replica/disabled) can be specified.+ */+ObjectAddress+CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString,+ Oid relOid, Oid refRelOid, Oid constraintOid,+ Oid indexOid, Oid funcoid, Oid parentTriggerOid,+ Node *whenClause, bool isInternal, bool in_partition,+ char trigger_fires_when)
{
int16 tgtype;
int ncolumns;
@@ -800,7 +818,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
CStringGetDatum(trigname));
values[Anum_pg_trigger_tgfoid - 1] = ObjectIdGetDatum(funcoid);
values[Anum_pg_trigger_tgtype - 1] = Int16GetDatum(tgtype);
- values[Anum_pg_trigger_tgenabled - 1] = CharGetDatum(TRIGGER_FIRES_ON_ORIGIN);+ values[Anum_pg_trigger_tgenabled - 1] = trigger_fires_when;
values[Anum_pg_trigger_tgisinternal - 1] = BoolGetDatum(isInternal || in_partition);
values[Anum_pg_trigger_tgconstrrelid - 1] = ObjectIdGetDatum(constrrelid);
values[Anum_pg_trigger_tgconstrindid - 1] = ObjectIdGetDatum(indexOid);
@@ -1130,11 +1148,11 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
map_partition_varattnos((List *) qual, PRS2_NEW_VARNO,
childTbl, rel);
- CreateTrigger(childStmt, queryString,- partdesc->oids[i], refRelOid,- InvalidOid, indexOnChild,- funcoid, trigoid, qual,- isInternal, true);+ CreateTriggerFiringOn(childStmt, queryString,+ partdesc->oids[i], refRelOid,+ InvalidOid, indexOnChild,+ funcoid, trigoid, qual,+ isInternal, true, trigger_fires_when);
table_close(childTbl, NoLock);
diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.hindex a40ddf5db5..40b8154876 100644--- a/src/include/commands/trigger.h+++ b/src/include/commands/trigger.h@@ -162,6 +162,11 @@ extern ObjectAddress CreateTrigger(CreateTrigStmt *stmt, const char *queryString
Oid relOid, Oid refRelOid, Oid constraintOid, Oid indexOid,
Oid funcoid, Oid parentTriggerOid, Node *whenClause,
bool isInternal, bool in_partition);
+extern ObjectAddress CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString,+ Oid relOid, Oid refRelOid, Oid constraintOid,+ Oid indexOid, Oid funcoid, Oid parentTriggerOid,+ Node *whenClause, bool isInternal, bool in_partition,+ char trigger_fires_when);
extern void RemoveTriggerById(Oid trigOid);
extern Oid get_trigger_oid(Oid relid, const char *name, bool missing_ok);
diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.outindex 5e76b3a47e..d85471a3a9 100644--- a/src/test/regress/expected/triggers.out+++ b/src/test/regress/expected/triggers.out@@ -2512,6 +2512,45 @@ select tgrelid::regclass, count(*) from pg_trigger
(5 rows)
drop table trg_clone;
+-- Verify that firing state propagates correctly+CREATE TABLE trgfire (i int) PARTITION BY RANGE (i);+CREATE TABLE trgfire1 PARTITION OF trgfire FOR VALUES FROM (1) TO (10);+CREATE OR REPLACE FUNCTION tgf() RETURNS trigger LANGUAGE plpgsql+ AS $$ begin raise exception 'except'; end $$;+CREATE TRIGGER tg AFTER INSERT ON trgfire FOR EACH ROW EXECUTE FUNCTION tgf();+INSERT INTO trgfire VALUES (1);+ERROR: except+CONTEXT: PL/pgSQL function tgf() line 1 at RAISE+ALTER TABLE trgfire DISABLE TRIGGER tg;+INSERT INTO trgfire VALUES (1);+CREATE TABLE trgfire2 PARTITION OF trgfire FOR VALUES FROM (10) TO (20);+INSERT INTO trgfire VALUES (11);+CREATE TABLE trgfire3 (LIKE trgfire);+ALTER TABLE trgfire ATTACH PARTITION trgfire3 FOR VALUES FROM (20) TO (30);+INSERT INTO trgfire VALUES (21);+SELECT tgrelid::regclass, tgenabled FROM pg_trigger+ WHERE tgrelid::regclass IN (SELECT relid FROM pg_partition_tree('trgfire'))+ ORDER BY tgrelid::regclass::text;+ tgrelid | tgenabled +----------+-----------+ trgfire | D+ trgfire1 | D+ trgfire2 | D+ trgfire3 | D+(4 rows)++ALTER TABLE trgfire ENABLE TRIGGER tg;+INSERT INTO trgfire VALUES (1);+ERROR: except+CONTEXT: PL/pgSQL function tgf() line 1 at RAISE+INSERT INTO trgfire VALUES (11);+ERROR: except+CONTEXT: PL/pgSQL function tgf() line 1 at RAISE+INSERT INTO trgfire VALUES (21);+ERROR: except+CONTEXT: PL/pgSQL function tgf() line 1 at RAISE+DROP TABLE trgfire;+DROP FUNCTION tgf();
--
-- Test the interaction between transition tables and both kinds of
-- inheritance. We'll dump the contents of the transition tables in a
diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sqlindex e228d0a8a5..cd5f85937e 100644--- a/src/test/regress/sql/triggers.sql+++ b/src/test/regress/sql/triggers.sql@@ -1749,6 +1749,30 @@ select tgrelid::regclass, count(*) from pg_trigger
group by tgrelid::regclass order by tgrelid::regclass;
drop table trg_clone;
+-- Verify that firing state propagates correctly+CREATE TABLE trgfire (i int) PARTITION BY RANGE (i);+CREATE TABLE trgfire1 PARTITION OF trgfire FOR VALUES FROM (1) TO (10);+CREATE OR REPLACE FUNCTION tgf() RETURNS trigger LANGUAGE plpgsql+ AS $$ begin raise exception 'except'; end $$;+CREATE TRIGGER tg AFTER INSERT ON trgfire FOR EACH ROW EXECUTE FUNCTION tgf();+INSERT INTO trgfire VALUES (1);+ALTER TABLE trgfire DISABLE TRIGGER tg;+INSERT INTO trgfire VALUES (1);+CREATE TABLE trgfire2 PARTITION OF trgfire FOR VALUES FROM (10) TO (20);+INSERT INTO trgfire VALUES (11);+CREATE TABLE trgfire3 (LIKE trgfire);+ALTER TABLE trgfire ATTACH PARTITION trgfire3 FOR VALUES FROM (20) TO (30);+INSERT INTO trgfire VALUES (21);+SELECT tgrelid::regclass, tgenabled FROM pg_trigger+ WHERE tgrelid::regclass IN (SELECT relid FROM pg_partition_tree('trgfire'))+ ORDER BY tgrelid::regclass::text;+ALTER TABLE trgfire ENABLE TRIGGER tg;+INSERT INTO trgfire VALUES (1);+INSERT INTO trgfire VALUES (11);+INSERT INTO trgfire VALUES (21);+DROP TABLE trgfire;+DROP FUNCTION tgf();+
--
-- Test the interaction between transition tables and both kinds of
-- inheritance. We'll dump the contents of the transition tables in a
--
2.20.1
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-hackers@postgresql.org
Cc: alvherre@alvh.no-ip.org, pryzby@telsasoft.com, amitlangote09@gmail.com
Subject: Re: CREATE TABLE .. PARTITION OF fails to preserve tgenabled for inherited row triggers
In-Reply-To: <20201016165934.GA5479@alvherre.pgsql>
* 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