Re: A row-level trigger on a partitioned table is not created on a sub-partition created later

Amit Langote <[email protected]> Wed, 18 Dec 2019 11:11:39 +0900
Newsgroups gmane.comp.db.postgresql.bugs
Message-ID <CA+HiwqFM2=i+uHB9o4OkLbE2S3sjPHoVe2wXuAD1GLJ4+Pk9eg@mail.gmail.com>
Hello,

On Thu, Dec 5, 2019 at 4:24 PM Petr Fedorov <[email protected]> wrote:
> create table level1 (id1 integer not null, id2 integer not null, id3 integer not null) partition by list (id2);
> create or replace function trigger_func() returns trigger language 'plpgsql' as $body$ begin raise exception 'fired'; return null; end $body$;
> create trigger test_trigger after insert on level1 for each row execute procedure trigger_func();
>
> create table level2 partition of level1 for values in (1) partition by list (id3);
> create table level3 partition of level2 for values in (1);
>
> insert into level1 values (1,1,1); -- row inserted
>
> psql \d+ shows that level3 does not have row level trigger while level2 and level1 have.

That is a bug. :(

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.

Thanks,
Amit
dont-mark-cloned-triggers-internal.patch (text/plain, 5.1 KB)
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index e9955707fa..9d23388100 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -1938,7 +1938,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 daa80ec4aa..1295f94f54 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -9853,7 +9853,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();
@@ -9921,7 +9921,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();
@@ -9977,7 +9977,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);
 }
 
 /*
@@ -16068,7 +16068,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 faeea16d21..3eeb00a81f 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);
@@ -1192,7 +1187,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/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);