From: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Thomas Munro <thomas.munro@enterprisedb.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Cc: Simon Riggs <simon@2ndquadrant.com>
Subject: Re: FOR EACH ROW triggers on partitioned tables
Date: Thu, 22 Mar 2018 12:27:00 -0400
Message-ID: <c847c761-dd14-9ea9-5aaa-1d3cf9606dd8@2ndquadrant.com> (raw)
In-Reply-To: <20180321231839.5zcjxvdgwrhgjwua@alvherre.pgsql>
References: <20180321231839.5zcjxvdgwrhgjwua@alvherre.pgsql>
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
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.hindex 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.hindex 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.outindex 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.sqlindex 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
Attachments:
[text/plain] 0001-fixup-Allow-FOR-EACH-ROW-triggers-on-partitioned-tab.patch (6.6K, ../c847c761-dd14-9ea9-5aaa-1d3cf9606dd8@2ndquadrant.com/2-0001-fixup-Allow-FOR-EACH-ROW-triggers-on-partitioned-tab.patch)
download | inline diff:
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.hindex 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.hindex 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.outindex 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.sqlindex 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
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: peter.eisentraut@2ndquadrant.com, alvherre@alvh.no-ip.org, thomas.munro@enterprisedb.com, robertmhaas@gmail.com, simon@2ndquadrant.com
Subject: Re: FOR EACH ROW triggers on partitioned tables
In-Reply-To: <c847c761-dd14-9ea9-5aaa-1d3cf9606dd8@2ndquadrant.com>
* 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