Re: A row-level trigger on a partitioned table is not created on a sub-partition created later
Amit Langote <[email protected]> Thu, 26 Dec 2019 17:00:34 +0900
| Newsgroups | gmane.comp.db.postgresql.bugs |
|---|---|
| Message-ID | <CA+HiwqE+7E1AOG+sN3KXwYTaJMU2J+k7v6mn+NJNfZhjZp9r5Q@mail.gmail.com> |
On Wed, Dec 18, 2019 at 6:56 PM Alvaro Herrera <[email protected]> wrote: > On 2019-Dec-18, Amit Langote wrote: > > Alvaro, isn't marking triggers cloned to partitions "internal" > > unnecessary? Because the cloned trigger on partition (level2 in above > > example) is marked internal, CloneRowTriggersToPartition() skips it > > when called on a sub-partition (level3 in above example). > > > > Attached patch to fix that passes make check, although a bit surprised > > that it does. > > IIRC that change would break pg_dump. Indeed it does, but looks like partition triggers are not tested that extensively in pg_dump's suite. Attached updated patch with pg_dump hacks seems to do the trick for me. What do you think? Thanks, Amit
dont-mark-cloned-triggers-internal_v2.patch
(text/plain, 6.9 KB)
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 7529bf39b8..f58a958d46 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -1937,7 +1937,7 @@ index_constraint_create(Relation heapRelation,
(void) CreateTrigger(trigger, NULL, RelationGetRelid(heapRelation),
InvalidOid, conOid, indexRelationId, InvalidOid,
- InvalidOid, NULL, true, false);
+ InvalidOid, NULL, true);
}
/*
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index e19772aa90..4ab4615fbe 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -9852,7 +9852,7 @@ CreateFKCheckTrigger(Oid myRelOid, Oid refRelOid, Constraint *fkconstraint,
fk_trigger->args = NIL;
(void) CreateTrigger(fk_trigger, NULL, myRelOid, refRelOid, constraintOid,
- indexOid, InvalidOid, InvalidOid, NULL, true, false);
+ indexOid, InvalidOid, InvalidOid, NULL, true);
/* Make changes-so-far visible */
CommandCounterIncrement();
@@ -9920,7 +9920,7 @@ createForeignKeyActionTriggers(Relation rel, Oid refRelOid, Constraint *fkconstr
(void) CreateTrigger(fk_trigger, NULL, refRelOid, RelationGetRelid(rel),
constraintOid,
- indexOid, InvalidOid, InvalidOid, NULL, true, false);
+ indexOid, InvalidOid, InvalidOid, NULL, true);
/* Make changes-so-far visible */
CommandCounterIncrement();
@@ -9976,7 +9976,7 @@ createForeignKeyActionTriggers(Relation rel, Oid refRelOid, Constraint *fkconstr
(void) CreateTrigger(fk_trigger, NULL, refRelOid, RelationGetRelid(rel),
constraintOid,
- indexOid, InvalidOid, InvalidOid, NULL, true, false);
+ indexOid, InvalidOid, InvalidOid, NULL, true);
}
/*
@@ -16057,7 +16057,7 @@ CloneRowTriggersToPartition(Relation parent, Relation partition)
CreateTrigger(trigStmt, NULL, RelationGetRelid(partition),
trigForm->tgconstrrelid, InvalidOid, InvalidOid,
trigForm->tgfoid, trigForm->oid, qual,
- false, true);
+ false);
MemoryContextSwitchTo(oldcxt);
MemoryContextReset(perTupCxt);
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index b9fca3af3c..a9e46988e8 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -162,7 +162,7 @@ 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)
+ bool isInternal)
{
int16 tgtype;
int ncolumns;
@@ -837,11 +837,6 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
/*
* Build the new pg_trigger tuple.
- *
- * When we're creating a trigger in a partition, we mark it as internal,
- * even though we don't do the isInternal magic in this function. This
- * makes the triggers in partitions identical to the ones in the
- * partitioned tables, except that they are marked internal.
*/
memset(nulls, false, sizeof(nulls));
@@ -852,7 +847,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
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_tgisinternal - 1] = BoolGetDatum(isInternal || in_partition);
+ values[Anum_pg_trigger_tgisinternal - 1] = BoolGetDatum(isInternal);
values[Anum_pg_trigger_tgconstrrelid - 1] = ObjectIdGetDatum(constrrelid);
values[Anum_pg_trigger_tgconstrindid - 1] = ObjectIdGetDatum(indexOid);
values[Anum_pg_trigger_tgconstraint - 1] = ObjectIdGetDatum(constraintOid);
@@ -1185,7 +1180,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
partdesc->oids[i], refRelOid,
InvalidOid, indexOnChild,
funcoid, trigoid, qual,
- isInternal, true);
+ isInternal);
table_close(childTbl, NoLock);
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 3a03ca7e2f..678d0e2d74 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1520,7 +1520,7 @@ ProcessUtilitySlow(ParseState *pstate,
address = CreateTrigger((CreateTrigStmt *) parsetree,
queryString, InvalidOid, InvalidOid,
InvalidOid, InvalidOid, InvalidOid,
- InvalidOid, NULL, false, false);
+ InvalidOid, NULL, false);
break;
case T_CreatePLangStmt:
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 08658c8e86..68f9e6639b 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -7616,13 +7616,39 @@ getTriggers(Archive *fout, TableInfo tblinfo[], int numTables)
tbinfo->dobj.name);
resetPQExpBuffer(query);
- if (fout->remoteVersion >= 90000)
+
+ /*
+ * For servers that support triggers on partitioned tables, must
+ * exclude triggers of partitions that are also defined in some
+ * partitioned ancestor.
+ *
+ * NB: think not to use pretty=true in pg_get_triggerdef. It
+ * could result in non-forward-compatible dumps of WHEN clauses
+ * due to under-parenthesization.
+ */
+ if (fout->remoteVersion >= 110000)
+ {
+ appendPQExpBuffer(query,
+ "SELECT tgname, "
+ "tgfoid::pg_catalog.regproc AS tgfname, "
+ "pg_catalog.pg_get_triggerdef(oid, false) AS tgdef, "
+ "tgenabled, tableoid, oid "
+ "FROM pg_catalog.pg_trigger t "
+ "WHERE tgrelid = '%u'::pg_catalog.oid "
+ "AND NOT tgisinternal "
+ "AND NOT EXISTS "
+ " (SELECT 1 FROM pg_trigger "
+ " WHERE tgname = t.tgname "
+ " AND tgrelid IN "
+ " (SELECT inhparent "
+ " FROM pg_inherits, pg_class c"
+ " WHERE inhrelid = t.tgrelid"
+ " AND t.tgrelid = c.oid"
+ " AND c.relispartition))",
+ tbinfo->dobj.catId.oid);
+ }
+ else if (fout->remoteVersion >= 90000)
{
- /*
- * NB: think not to use pretty=true in pg_get_triggerdef. It
- * could result in non-forward-compatible dumps of WHEN clauses
- * due to under-parenthesization.
- */
appendPQExpBuffer(query,
"SELECT tgname, "
"tgfoid::pg_catalog.regproc AS tgfname, "
diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h
index a46feeedb0..633d75bd49 100644
--- a/src/include/commands/trigger.h
+++ b/src/include/commands/trigger.h
@@ -160,7 +160,7 @@ extern PGDLLIMPORT int SessionReplicationRole;
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);
+ bool isInternal);
extern void RemoveTriggerById(Oid trigOid);
extern Oid get_trigger_oid(Oid relid, const char *name, bool missing_ok);