From 52e0de2aa3ac6941c429a237ad3123902cda5f10 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Tue, 21 Nov 2017 15:53:11 -0300
Subject: [PATCH v7 3/4] Allow FOR EACH ROW triggers on partitioned tables

---
 src/backend/commands/trigger.c         | 73 +++++++++++++++++++++++++++++++---
 src/test/regress/expected/triggers.out | 55 ++++++++++++++++++++-----
 src/test/regress/sql/triggers.sql      | 42 ++++++++++++++-----
 3 files changed, 142 insertions(+), 28 deletions(-)

diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 92ae3822d8..15a3cd0bff 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -133,6 +133,10 @@ static bool before_stmt_triggers_fired(Oid relid, CmdType cmdType);
  * relation, as well as ACL_EXECUTE on the trigger function.  For internal
  * triggers the caller must apply any required permission checks.
  *
+ * When called on partitioned tables, this function recurses to create the
+ * trigger on all the partitions, except if isInternal is true, in which
+ * case caller is expected to execute recursion on its own.
+ *
  * Note: can return InvalidObjectAddress if we decided to not create a trigger
  * at all, but a foreign-key constraint.  This is a kluge for backwards
  * compatibility.
@@ -179,8 +183,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
 	 * Triggers must be on tables or views, and there are additional
 	 * relation-type-specific restrictions.
 	 */
-	if (rel->rd_rel->relkind == RELKIND_RELATION ||
-		rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+	if (rel->rd_rel->relkind == RELKIND_RELATION)
 	{
 		/* Tables can't have INSTEAD OF triggers */
 		if (stmt->timing != TRIGGER_TYPE_BEFORE &&
@@ -190,13 +193,46 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
 					 errmsg("\"%s\" is a table",
 							RelationGetRelationName(rel)),
 					 errdetail("Tables cannot have INSTEAD OF triggers.")));
-		/* Disallow ROW triggers on partitioned tables */
-		if (stmt->row && rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+	}
+	else if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+	{
+		/* Partitioned tables can't have INSTEAD OF triggers */
+		if (stmt->timing != TRIGGER_TYPE_BEFORE &&
+			stmt->timing != TRIGGER_TYPE_AFTER)
 			ereport(ERROR,
 					(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-					 errmsg("\"%s\" is a partitioned table",
+					 errmsg("\"%s\" is a table",
 							RelationGetRelationName(rel)),
-					 errdetail("Partitioned tables cannot have ROW triggers.")));
+					 errdetail("Tables cannot have INSTEAD OF triggers.")));
+
+		/*
+		 * FOR EACH ROW triggers have further restrictions
+		 */
+		if (stmt->row)
+		{
+			/*
+			 * Disallow WHEN clauses; I think it's okay, but disallow for now
+			 * to reduce testing surface.
+			 */
+			if (stmt->whenClause)
+				ereport(ERROR,
+						(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+						 errmsg("\"%s\" is a partitioned table",
+								RelationGetRelationName(rel)),
+						 errdetail("Triggers FOR EACH ROW on partitioned table cannot have WHEN clauses.")));
+
+			/*
+			 * BEFORE triggers FOR EACH ROW are forbidden, because they would
+			 * allow the user to direct the row to another partition, which
+			 * isn't implemented in the executor.
+			 */
+			if (stmt->timing != TRIGGER_TYPE_AFTER)
+				ereport(ERROR,
+						(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+						 errmsg("\"%s\" is a partitioned table",
+								RelationGetRelationName(rel)),
+						 errdetail("Partitioned tables cannot have BEFORE / FOR EACH ROW triggers.")));
+		}
 	}
 	else if (rel->rd_rel->relkind == RELKIND_VIEW)
 	{
@@ -982,6 +1018,31 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
 	InvokeObjectPostCreateHookArg(TriggerRelationId, trigoid, 0,
 								  isInternal);
 
+	/*
+	 * If this is a FOR EACH ROW trigger on a partitioned table, recurse for
+	 * each partition if invoked directly by user (otherwise, caller must do
+	 * its own recursion).
+	 */
+	if (stmt->row && rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE &&
+		!isInternal)
+	{
+		PartitionDesc	partdesc = RelationGetPartitionDesc(rel);
+		int				i;
+
+		for (i = 0; i < partdesc->nparts; i++)
+		{
+			/* XXX must create a separate constraint for each child */
+			Assert(constraintOid == InvalidOid);
+			/* XXX must create a separate index for each child */
+			Assert(indexOid == InvalidOid);
+
+			CreateTrigger(copyObject(stmt), queryString,
+						  partdesc->oids[i], refRelOid,
+						  constraintOid, indexOid,
+						  isInternal);
+		}
+	}
+
 	/* Keep lock on target rel until end of xact */
 	heap_close(rel, NoLock);
 
diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out
index 85d948741e..e3a88aceca 100644
--- a/src/test/regress/expected/triggers.out
+++ b/src/test/regress/expected/triggers.out
@@ -1824,7 +1824,7 @@ create or replace function trigger_notice() returns trigger as $$
     return null;
   end;
   $$ language plpgsql;
--- insert/update/delete statment-level triggers on the parent
+-- insert/update/delete statement-level triggers on the parent
 create trigger trig_ins_before before insert on parted_stmt_trig
   for each statement execute procedure trigger_notice();
 create trigger trig_ins_after after insert on parted_stmt_trig
@@ -1837,27 +1837,51 @@ create trigger trig_del_before before delete on parted_stmt_trig
   for each statement execute procedure trigger_notice();
 create trigger trig_del_after after delete on parted_stmt_trig
   for each statement execute procedure trigger_notice();
+-- these cases are disallowed
+create trigger trig_ins_before_1 before insert on parted_stmt_trig
+  for each row execute procedure trigger_notice();
+ERROR:  "parted_stmt_trig" is a partitioned table
+DETAIL:  Partitioned tables cannot have BEFORE / FOR EACH ROW triggers.
+create trigger trig_upd_before_1 before update on parted_stmt_trig
+  for each row execute procedure trigger_notice();
+ERROR:  "parted_stmt_trig" is a partitioned table
+DETAIL:  Partitioned tables cannot have BEFORE / FOR EACH ROW triggers.
+create trigger trig_del_before_1 before delete on parted_stmt_trig
+  for each row execute procedure trigger_notice();
+ERROR:  "parted_stmt_trig" is a partitioned table
+DETAIL:  Partitioned tables cannot have BEFORE / FOR EACH ROW triggers.
+-- insert/update/delete row-level triggers on the parent
+create trigger trig_ins_after_parent after insert on parted_stmt_trig
+  for each row execute procedure trigger_notice();
+create trigger trig_upd_after_parent after update on parted_stmt_trig
+  for each row execute procedure trigger_notice();
+create trigger trig_del_after_parent after delete on parted_stmt_trig
+  for each row execute procedure trigger_notice();
 -- insert/update/delete row-level triggers on the first partition
-create trigger trig_ins_before before insert on parted_stmt_trig1
+create trigger trig_ins_before_child before insert on parted_stmt_trig1
   for each row execute procedure trigger_notice();
-create trigger trig_ins_after after insert on parted_stmt_trig1
+create trigger trig_ins_after_child after insert on parted_stmt_trig1
   for each row execute procedure trigger_notice();
-create trigger trig_upd_before before update on parted_stmt_trig1
+create trigger trig_upd_before_child before update on parted_stmt_trig1
   for each row execute procedure trigger_notice();
-create trigger trig_upd_after after update on parted_stmt_trig1
+create trigger trig_upd_after_child after update on parted_stmt_trig1
+  for each row execute procedure trigger_notice();
+create trigger trig_del_before_child before delete on parted_stmt_trig1
+  for each row execute procedure trigger_notice();
+create trigger trig_del_after_child after delete on parted_stmt_trig1
   for each row execute procedure trigger_notice();
 -- insert/update/delete statement-level triggers on the parent
-create trigger trig_ins_before before insert on parted2_stmt_trig
+create trigger trig_ins_before_3 before insert on parted2_stmt_trig
   for each statement execute procedure trigger_notice();
-create trigger trig_ins_after after insert on parted2_stmt_trig
+create trigger trig_ins_after_3 after insert on parted2_stmt_trig
   for each statement execute procedure trigger_notice();
-create trigger trig_upd_before before update on parted2_stmt_trig
+create trigger trig_upd_before_3 before update on parted2_stmt_trig
   for each statement execute procedure trigger_notice();
-create trigger trig_upd_after after update on parted2_stmt_trig
+create trigger trig_upd_after_3 after update on parted2_stmt_trig
   for each statement execute procedure trigger_notice();
-create trigger trig_del_before before delete on parted2_stmt_trig
+create trigger trig_del_before_3 before delete on parted2_stmt_trig
   for each statement execute procedure trigger_notice();
-create trigger trig_del_after after delete on parted2_stmt_trig
+create trigger trig_del_after_3 after delete on parted2_stmt_trig
   for each statement execute procedure trigger_notice();
 with ins (a) as (
   insert into parted2_stmt_trig values (1), (2) returning a
@@ -1866,6 +1890,8 @@ NOTICE:  trigger on parted_stmt_trig BEFORE INSERT for STATEMENT
 NOTICE:  trigger on parted2_stmt_trig BEFORE INSERT for STATEMENT
 NOTICE:  trigger on parted_stmt_trig1 BEFORE INSERT for ROW
 NOTICE:  trigger on parted_stmt_trig1 AFTER INSERT for ROW
+NOTICE:  trigger on parted_stmt_trig1 AFTER INSERT for ROW
+NOTICE:  trigger on parted_stmt_trig2 AFTER INSERT for ROW
 NOTICE:  trigger on parted2_stmt_trig AFTER INSERT for STATEMENT
 NOTICE:  trigger on parted_stmt_trig AFTER INSERT for STATEMENT
      tableoid      | a 
@@ -1881,21 +1907,28 @@ NOTICE:  trigger on parted_stmt_trig BEFORE UPDATE for STATEMENT
 NOTICE:  trigger on parted_stmt_trig1 BEFORE UPDATE for ROW
 NOTICE:  trigger on parted2_stmt_trig BEFORE UPDATE for STATEMENT
 NOTICE:  trigger on parted_stmt_trig1 AFTER UPDATE for ROW
+NOTICE:  trigger on parted_stmt_trig1 AFTER UPDATE for ROW
+NOTICE:  trigger on parted_stmt_trig2 AFTER UPDATE for ROW
 NOTICE:  trigger on parted_stmt_trig AFTER UPDATE for STATEMENT
 NOTICE:  trigger on parted2_stmt_trig AFTER UPDATE for STATEMENT
 delete from parted_stmt_trig;
 NOTICE:  trigger on parted_stmt_trig BEFORE DELETE for STATEMENT
+NOTICE:  trigger on parted_stmt_trig1 BEFORE DELETE for ROW
+NOTICE:  trigger on parted_stmt_trig2 AFTER DELETE for ROW
 NOTICE:  trigger on parted_stmt_trig AFTER DELETE for STATEMENT
 -- insert via copy on the parent
 copy parted_stmt_trig(a) from stdin;
 NOTICE:  trigger on parted_stmt_trig BEFORE INSERT for STATEMENT
 NOTICE:  trigger on parted_stmt_trig1 BEFORE INSERT for ROW
 NOTICE:  trigger on parted_stmt_trig1 AFTER INSERT for ROW
+NOTICE:  trigger on parted_stmt_trig1 AFTER INSERT for ROW
+NOTICE:  trigger on parted_stmt_trig2 AFTER INSERT for ROW
 NOTICE:  trigger on parted_stmt_trig AFTER INSERT for STATEMENT
 -- insert via copy on the first partition
 copy parted_stmt_trig1(a) from stdin;
 NOTICE:  trigger on parted_stmt_trig1 BEFORE INSERT for ROW
 NOTICE:  trigger on parted_stmt_trig1 AFTER INSERT for ROW
+NOTICE:  trigger on parted_stmt_trig1 AFTER INSERT for ROW
 drop table parted_stmt_trig, parted2_stmt_trig;
 --
 -- Test the interaction between transition tables and both kinds of
diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql
index 2b2236ed7d..7b1f17cf9a 100644
--- a/src/test/regress/sql/triggers.sql
+++ b/src/test/regress/sql/triggers.sql
@@ -1307,7 +1307,7 @@ create or replace function trigger_notice() returns trigger as $$
   end;
   $$ language plpgsql;
 
--- insert/update/delete statment-level triggers on the parent
+-- insert/update/delete statement-level triggers on the parent
 create trigger trig_ins_before before insert on parted_stmt_trig
   for each statement execute procedure trigger_notice();
 create trigger trig_ins_after after insert on parted_stmt_trig
@@ -1321,28 +1321,48 @@ create trigger trig_del_before before delete on parted_stmt_trig
 create trigger trig_del_after after delete on parted_stmt_trig
   for each statement execute procedure trigger_notice();
 
+-- these cases are disallowed
+create trigger trig_ins_before_1 before insert on parted_stmt_trig
+  for each row execute procedure trigger_notice();
+create trigger trig_upd_before_1 before update on parted_stmt_trig
+  for each row execute procedure trigger_notice();
+create trigger trig_del_before_1 before delete on parted_stmt_trig
+  for each row execute procedure trigger_notice();
+
+-- insert/update/delete row-level triggers on the parent
+create trigger trig_ins_after_parent after insert on parted_stmt_trig
+  for each row execute procedure trigger_notice();
+create trigger trig_upd_after_parent after update on parted_stmt_trig
+  for each row execute procedure trigger_notice();
+create trigger trig_del_after_parent after delete on parted_stmt_trig
+  for each row execute procedure trigger_notice();
+
 -- insert/update/delete row-level triggers on the first partition
-create trigger trig_ins_before before insert on parted_stmt_trig1
+create trigger trig_ins_before_child before insert on parted_stmt_trig1
   for each row execute procedure trigger_notice();
-create trigger trig_ins_after after insert on parted_stmt_trig1
+create trigger trig_ins_after_child after insert on parted_stmt_trig1
   for each row execute procedure trigger_notice();
-create trigger trig_upd_before before update on parted_stmt_trig1
+create trigger trig_upd_before_child before update on parted_stmt_trig1
   for each row execute procedure trigger_notice();
-create trigger trig_upd_after after update on parted_stmt_trig1
+create trigger trig_upd_after_child after update on parted_stmt_trig1
+  for each row execute procedure trigger_notice();
+create trigger trig_del_before_child before delete on parted_stmt_trig1
+  for each row execute procedure trigger_notice();
+create trigger trig_del_after_child after delete on parted_stmt_trig1
   for each row execute procedure trigger_notice();
 
 -- insert/update/delete statement-level triggers on the parent
-create trigger trig_ins_before before insert on parted2_stmt_trig
+create trigger trig_ins_before_3 before insert on parted2_stmt_trig
   for each statement execute procedure trigger_notice();
-create trigger trig_ins_after after insert on parted2_stmt_trig
+create trigger trig_ins_after_3 after insert on parted2_stmt_trig
   for each statement execute procedure trigger_notice();
-create trigger trig_upd_before before update on parted2_stmt_trig
+create trigger trig_upd_before_3 before update on parted2_stmt_trig
   for each statement execute procedure trigger_notice();
-create trigger trig_upd_after after update on parted2_stmt_trig
+create trigger trig_upd_after_3 after update on parted2_stmt_trig
   for each statement execute procedure trigger_notice();
-create trigger trig_del_before before delete on parted2_stmt_trig
+create trigger trig_del_before_3 before delete on parted2_stmt_trig
   for each statement execute procedure trigger_notice();
-create trigger trig_del_after after delete on parted2_stmt_trig
+create trigger trig_del_after_3 after delete on parted2_stmt_trig
   for each statement execute procedure trigger_notice();
 
 with ins (a) as (
-- 
2.11.0

