Subject: Re: FOR EACH ROW triggers on partitioned tables
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
References: <20180321231839.5zcjxvdgwrhgjwua@alvherre.pgsql>
Cc: Thomas Munro <thomas.munro@enterprisedb.com>,
 Robert Haas <robertmhaas@gmail.com>,
 Pg Hackers <pgsql-hackers@postgresql.org>,
 Simon Riggs <simon@2ndquadrant.com>
From: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Organization: 2ndQuadrant
Message-ID: <c847c761-dd14-9ea9-5aaa-1d3cf9606dd8@2ndquadrant.com>
Date: Thu, 22 Mar 2018 12:27:00 -0400
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:52.0)
 Gecko/20100101 Thunderbird/52.6.0
MIME-Version: 1.0
In-Reply-To: <20180321231839.5zcjxvdgwrhgjwua@alvherre.pgsql>
Content-Type: multipart/mixed;
 boundary="------------B66B6AFF9463E5DA9B36B7A2"
Content-Language: en-US
Precedence: bulk

This is a multi-part message in MIME format.
--------------B66B6AFF9463E5DA9B36B7A2
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit

On 3/21/18 19:18, Alvaro Herrera wrote:
> Here's v8, which addresses all your comments except the doc updates.  I
> added a few more tests, too.  Thanks for the review!  I intend to commit
> this shortly, probably not before Friday to give some more time for
> others to review/comment.

Looks good, does what it needs to do.

A small fixup attached.  In particular, I renamed one trigger from "f",
which created confusing output, looking like a boolean column.

This comment in the tests I don't understand:

-- verify that the FOR UPDATE OF (columns) is propagated correctly

I don't see how this applies to the tests that follow.  Does this have
something to do with the subsequent foreign keys patch perhaps?

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--------------B66B6AFF9463E5DA9B36B7A2
Content-Type: text/plain; charset=UTF-8; x-mac-type="0"; x-mac-creator="0";
 name="0001-fixup-Allow-FOR-EACH-ROW-triggers-on-partitioned-tab.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename*0="0001-fixup-Allow-FOR-EACH-ROW-triggers-on-partitioned-tab.pa";
 filename*1="tch"

From 37c41e1be7fbc1a02c7d543a471c84aee7b75a9f Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Thu, 22 Mar 2018 12:07:54 -0400
Subject: [PATCH] fixup! Allow FOR EACH ROW triggers on partitioned tables

---
 src/include/catalog/pg_constraint.h    |  2 +-
 src/include/commands/trigger.h         |  2 +-
 src/test/regress/expected/triggers.out | 36 +++++++++++++++++-----------------
 src/test/regress/sql/triggers.sql      | 10 +++++-----
 4 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/src/include/catalog/pg_constraint.h b/src/include/catalog/pg_constraint.h
index 45b26cdfa8..3957e07235 100644
--- a/src/include/catalog/pg_constraint.h
+++ b/src/include/catalog/pg_constraint.h
@@ -73,7 +73,7 @@ CATALOG(pg_constraint,2606)
 	Oid			conindid;		/* index supporting this constraint */
 
 	/*
-	 * if this constraint is on a partition inherited from a partitioned table,
+	 * If this constraint is on a partition inherited from a partitioned table,
 	 * this is the OID of the corresponding constraint in the parent.
 	 */
 	Oid			conparentid;
diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h
index 2a6f2cd934..a5b8610fa2 100644
--- a/src/include/commands/trigger.h
+++ b/src/include/commands/trigger.h
@@ -159,7 +159,7 @@ extern PGDLLIMPORT int SessionReplicationRole;
 
 extern ObjectAddress CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
 			  Oid relOid, Oid refRelOid, Oid constraintOid, Oid indexOid,
-			  Oid funcid, Oid parentTriggerOid, Node *whenClause,
+			  Oid funcoid, Oid parentTriggerOid, Node *whenClause,
 			  bool isInternal, bool in_partition);
 
 extern void RemoveTriggerById(Oid trigOid);
diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out
index 2cb56efdf9..a4e9ea03f3 100644
--- a/src/test/regress/expected/triggers.out
+++ b/src/test/regress/expected/triggers.out
@@ -1871,7 +1871,7 @@ drop table parted_trig;
 --
 create table trigpart (a int, b int) partition by range (a);
 create table trigpart1 partition of trigpart for values from (0) to (1000);
-create trigger f after insert on trigpart for each row execute procedure trigger_nothing();
+create trigger trg1 after insert on trigpart for each row execute procedure trigger_nothing();
 create table trigpart2 partition of trigpart for values from (1000) to (2000);
 create table trigpart3 (like trigpart);
 alter table trigpart attach partition trigpart3 for values from (2000) to (3000);
@@ -1879,32 +1879,32 @@ select tgrelid::regclass, tgname, tgfoid::regproc from pg_trigger
   where tgrelid::regclass::text like 'trigpart%' order by tgrelid::regclass::text;
   tgrelid  | tgname |     tgfoid      
 -----------+--------+-----------------
- trigpart  | f      | trigger_nothing
- trigpart1 | f      | trigger_nothing
- trigpart2 | f      | trigger_nothing
- trigpart3 | f      | trigger_nothing
+ trigpart  | trg1   | trigger_nothing
+ trigpart1 | trg1   | trigger_nothing
+ trigpart2 | trg1   | trigger_nothing
+ trigpart3 | trg1   | trigger_nothing
 (4 rows)
 
-drop trigger f on trigpart1;	-- fail
-ERROR:  cannot drop trigger f on table trigpart1 because trigger f on table trigpart requires it
-HINT:  You can drop trigger f on table trigpart instead.
-drop trigger f on trigpart2;	-- fail
-ERROR:  cannot drop trigger f on table trigpart2 because trigger f on table trigpart requires it
-HINT:  You can drop trigger f on table trigpart instead.
-drop trigger f on trigpart3;	-- fail
-ERROR:  cannot drop trigger f on table trigpart3 because trigger f on table trigpart requires it
-HINT:  You can drop trigger f on table trigpart instead.
+drop trigger trg1 on trigpart1;	-- fail
+ERROR:  cannot drop trigger trg1 on table trigpart1 because trigger trg1 on table trigpart requires it
+HINT:  You can drop trigger trg1 on table trigpart instead.
+drop trigger trg1 on trigpart2;	-- fail
+ERROR:  cannot drop trigger trg1 on table trigpart2 because trigger trg1 on table trigpart requires it
+HINT:  You can drop trigger trg1 on table trigpart instead.
+drop trigger trg1 on trigpart3;	-- fail
+ERROR:  cannot drop trigger trg1 on table trigpart3 because trigger trg1 on table trigpart requires it
+HINT:  You can drop trigger trg1 on table trigpart instead.
 drop table trigpart2;			-- ok, trigger should be gone in that partition
 select tgrelid::regclass, tgname, tgfoid::regproc from pg_trigger
   where tgrelid::regclass::text like 'trigpart%' order by tgrelid::regclass::text;
   tgrelid  | tgname |     tgfoid      
 -----------+--------+-----------------
- trigpart  | f      | trigger_nothing
- trigpart1 | f      | trigger_nothing
- trigpart3 | f      | trigger_nothing
+ trigpart  | trg1   | trigger_nothing
+ trigpart1 | trg1   | trigger_nothing
+ trigpart3 | trg1   | trigger_nothing
 (3 rows)
 
-drop trigger f on trigpart;		-- ok, all gone
+drop trigger trg1 on trigpart;		-- ok, all gone
 select tgrelid::regclass, tgname, tgfoid::regproc from pg_trigger
   where tgrelid::regclass::text like 'trigpart%' order by tgrelid::regclass::text;
  tgrelid | tgname | tgfoid 
diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql
index 4d35f39f29..edb7a2f978 100644
--- a/src/test/regress/sql/triggers.sql
+++ b/src/test/regress/sql/triggers.sql
@@ -1305,19 +1305,19 @@ CREATE RULE european_city_delete_rule AS ON DELETE TO european_city_view
 --
 create table trigpart (a int, b int) partition by range (a);
 create table trigpart1 partition of trigpart for values from (0) to (1000);
-create trigger f after insert on trigpart for each row execute procedure trigger_nothing();
+create trigger trg1 after insert on trigpart for each row execute procedure trigger_nothing();
 create table trigpart2 partition of trigpart for values from (1000) to (2000);
 create table trigpart3 (like trigpart);
 alter table trigpart attach partition trigpart3 for values from (2000) to (3000);
 select tgrelid::regclass, tgname, tgfoid::regproc from pg_trigger
   where tgrelid::regclass::text like 'trigpart%' order by tgrelid::regclass::text;
-drop trigger f on trigpart1;	-- fail
-drop trigger f on trigpart2;	-- fail
-drop trigger f on trigpart3;	-- fail
+drop trigger trg1 on trigpart1;	-- fail
+drop trigger trg1 on trigpart2;	-- fail
+drop trigger trg1 on trigpart3;	-- fail
 drop table trigpart2;			-- ok, trigger should be gone in that partition
 select tgrelid::regclass, tgname, tgfoid::regproc from pg_trigger
   where tgrelid::regclass::text like 'trigpart%' order by tgrelid::regclass::text;
-drop trigger f on trigpart;		-- ok, all gone
+drop trigger trg1 on trigpart;		-- ok, all gone
 select tgrelid::regclass, tgname, tgfoid::regproc from pg_trigger
   where tgrelid::regclass::text like 'trigpart%' order by tgrelid::regclass::text;
 
-- 
2.16.2


--------------B66B6AFF9463E5DA9B36B7A2--