pg.ddx.io pgsql-hackers@postgresql.org mailing list archive help / color / mirror / Atom feed 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 10:52:30 -0300
Message-ID: <20201016135230.GA23633@alvherre.pgsql> (raw )
In-Reply-To: <20200930223450.GA14848@telsasoft.com >
On 2020-Sep-30, Justin Pryzby wrote:
> postgres=# SELECT tgrelid::regclass, tgenabled FROM pg_trigger WHERE tgrelid::regclass::text IN ('t1','t2');
> tgrelid | tgenabled
> ---------+-----------
> t1 | D
> t2 | O
> (2 rows)
>
> I consider this a bug,
Yeah.
> but CreateTrigStmt doesn't have any "enabled" member
> (since it's impossible to CREATE TRIGGER .. DISABLED), so I'm not sure where
> the fix should be.
I suggest we add a new function, as in the attached. It's important to
keep the ABI of CreateTrigger unchanged, for the sake of
backpatchability, but ISTM it's equally important to keep its API
unchanged, because outside callers such as ProcessUtility_slow does not
have to care about the new trigger's enabled state.
Attachments:
disable-triggers.patch (text/x-diff, 4.0K) download
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 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.c
index 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.h
index 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);
view thread (24+ messages) latest in thread
Message-ID: <20201016135230.GA23633@alvherre.pgsql>
Permalink: ../20201016135230.GA23633@alvherre.pgsql/
Also on: postgresql.org/message-id/20201016135230.GA23633@alvherre.pgsql
copy link · copy postgr.es · copy git am
reply 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: <20201016135230.GA23633@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