Transition tables for triggers on foreign tables and views
Hi hackers,
My colleague Prabhat Sahu reported off list that transition tables
don't work for views. I probably should have thought about that when
I fixed something similar for partitioned tables, and after some
experimentation I see that this is also broken for foreign tables.
For foreign tables using postgres_fdw, I see that transition tables
capture new rows for INSERT but capture nothing for DELETE and UPDATE.
For views, aside from the question of transition tables, I noticed
that statement triggers don't seem to fire at all with updatable
views. Surely they should -- isn't that a separate bug?
Example:
create table my_table (i int);
create view my_view as select * from my_table;
create function my_trigger_function() returns trigger language plpgsql as
$$ begin raise warning 'hello world'; return null; end; $$;
create trigger my_trigger after insert or update or delete on my_view
for each statement execute procedure my_trigger_function();
insert into my_view values (42);
... and the world remains ungreeted.
As for transition tables, there are probably meaningful ways to
support those for both views and foreign tables at least in certain
cases, as future feature enhancements. For now, do you agree that we
should reject such triggers as unsupported? See attached.
Separately, I noticed an obsolete sentence in the trigger
documentation. See doc.patch.
--
Thomas Munro
http://www.enterprisedb.com
Attachments:
prevent-unsupported-transition-tables.patchapplication/octet-stream; name=prevent-unsupported-transition-tables.patchDownload
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index d05e51c820..c1ac5f93db 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -361,6 +361,20 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
RelationGetRelationName(rel)),
errdetail("Triggers on partitioned tables cannot have transition tables.")));
+ if (rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("\"%s\" is a foreign table",
+ RelationGetRelationName(rel)),
+ errdetail("Transition tables are not supported for triggers on foreign tables.")));
+
+ if (rel->rd_rel->relkind == RELKIND_VIEW)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("\"%s\" is a view",
+ RelationGetRelationName(rel)),
+ errdetail("Transition tables are not supported for triggers on views.")));
+
if (stmt->timing != TRIGGER_TYPE_AFTER)
ereport(ERROR,
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out
index 1c7a7593f9..dcf2561eea 100644
--- a/src/test/regress/expected/foreign_data.out
+++ b/src/test/regress/expected/foreign_data.out
@@ -1215,6 +1215,13 @@ CREATE TRIGGER trigtest_after_stmt AFTER INSERT OR UPDATE OR DELETE
ON foreign_schema.foreign_table_1
FOR EACH STATEMENT
EXECUTE PROCEDURE dummy_trigger();
+CREATE TRIGGER trigtest_after_stmt_tt AFTER INSERT OR UPDATE OR DELETE -- ERROR
+ON foreign_schema.foreign_table_1
+REFERENCING NEW TABLE AS new_table
+FOR EACH STATEMENT
+EXECUTE PROCEDURE dummy_trigger();
+ERROR: "foreign_table_1" is a foreign table
+DETAIL: Transition tables are not supported for triggers on foreign tables.
CREATE TRIGGER trigtest_before_row BEFORE INSERT OR UPDATE OR DELETE
ON foreign_schema.foreign_table_1
FOR EACH ROW
diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out
index 4b0b3b7c42..f3814292bf 100644
--- a/src/test/regress/expected/triggers.out
+++ b/src/test/regress/expected/triggers.out
@@ -1785,5 +1785,19 @@ drop trigger my_trigger on my_table_42;
create trigger my_trigger after update on my_table_42 referencing old table as old_table
for each statement execute procedure my_trigger_function();
drop trigger my_trigger on my_table_42;
+drop function my_trigger_function();
drop table my_table_42;
drop table my_table;
+--
+-- Verify that triggers are not allowed on views
+--
+create table my_table (i int);
+create view my_view as select * from my_table;
+create function my_trigger_function() returns trigger as $$ begin end; $$ language plpgsql;
+create trigger my_trigger after update on my_view referencing old table as old_table
+ for each statement execute procedure my_trigger_function();
+ERROR: "my_view" is a view
+DETAIL: Transition tables are not supported for triggers on views.
+drop function my_trigger_function();
+drop view my_view;
+drop table my_table;
diff --git a/src/test/regress/sql/foreign_data.sql b/src/test/regress/sql/foreign_data.sql
index aaf079cf52..b3614a8233 100644
--- a/src/test/regress/sql/foreign_data.sql
+++ b/src/test/regress/sql/foreign_data.sql
@@ -512,6 +512,12 @@ ON foreign_schema.foreign_table_1
FOR EACH STATEMENT
EXECUTE PROCEDURE dummy_trigger();
+CREATE TRIGGER trigtest_after_stmt_tt AFTER INSERT OR UPDATE OR DELETE -- ERROR
+ON foreign_schema.foreign_table_1
+REFERENCING NEW TABLE AS new_table
+FOR EACH STATEMENT
+EXECUTE PROCEDURE dummy_trigger();
+
CREATE TRIGGER trigtest_before_row BEFORE INSERT OR UPDATE OR DELETE
ON foreign_schema.foreign_table_1
FOR EACH ROW
diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql
index 4473ce0518..189ff0e927 100644
--- a/src/test/regress/sql/triggers.sql
+++ b/src/test/regress/sql/triggers.sql
@@ -1261,5 +1261,19 @@ drop trigger my_trigger on my_table_42;
create trigger my_trigger after update on my_table_42 referencing old table as old_table
for each statement execute procedure my_trigger_function();
drop trigger my_trigger on my_table_42;
+drop function my_trigger_function();
drop table my_table_42;
drop table my_table;
+
+--
+-- Verify that triggers are not allowed on views
+--
+
+create table my_table (i int);
+create view my_view as select * from my_table;
+create function my_trigger_function() returns trigger as $$ begin end; $$ language plpgsql;
+create trigger my_trigger after update on my_view referencing old table as old_table
+ for each statement execute procedure my_trigger_function();
+drop function my_trigger_function();
+drop view my_view;
+drop table my_table;
doc.patchapplication/octet-stream; name=doc.patchDownload
diff --git a/doc/src/sgml/trigger.sgml b/doc/src/sgml/trigger.sgml
index 2a718d7f47..5c859ccdd8 100644
--- a/doc/src/sgml/trigger.sgml
+++ b/doc/src/sgml/trigger.sgml
@@ -305,8 +305,9 @@
<varname>NEW</varname> row for <command>INSERT</command> and
<command>UPDATE</command> triggers, and/or the <varname>OLD</varname> row
for <command>UPDATE</command> and <command>DELETE</command> triggers.
- Statement-level triggers do not currently have any way to examine the
- individual row(s) modified by the statement.
+ Statement-level <literal>AFTER</literal> triggers can optionally examine
+ the set of rows modified by the statement using either SQL queries or
+ tuplestores.
</para>
</sect1>
On Wed, Apr 26, 2017 at 11:17:05AM +1200, Thomas Munro wrote:
My colleague Prabhat Sahu reported off list that transition tables
don't work for views. I probably should have thought about that when
I fixed something similar for partitioned tables, and after some
experimentation I see that this is also broken for foreign tables.For foreign tables using postgres_fdw, I see that transition tables
capture new rows for INSERT but capture nothing for DELETE and UPDATE.For views, aside from the question of transition tables, I noticed
that statement triggers don't seem to fire at all with updatable
views. Surely they should -- isn't that a separate bug?Example:
create table my_table (i int);
create view my_view as select * from my_table;
create function my_trigger_function() returns trigger language plpgsql as
$$ begin raise warning 'hello world'; return null; end; $$;
create trigger my_trigger after insert or update or delete on my_view
for each statement execute procedure my_trigger_function();
insert into my_view values (42);... and the world remains ungreeted.
As for transition tables, there are probably meaningful ways to
support those for both views and foreign tables at least in certain
cases, as future feature enhancements. For now, do you agree that we
should reject such triggers as unsupported? See attached.
[Action required within three days. This is a generic notification.]
The above-described topic is currently a PostgreSQL 10 open item. Kevin,
since you committed the patch believed to have created it, you own this open
item. If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know. Otherwise, please observe the policy on
open item ownership[1]/messages/by-id/20170404140717.GA2675809@tornado.leadboat.com and send a status update within three calendar days of
this message. Include a date for your subsequent status update. Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10. Consequently, I will appreciate your efforts
toward speedy resolution. Thanks.
[1]: /messages/by-id/20170404140717.GA2675809@tornado.leadboat.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Apr 25, 2017 at 6:17 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
My colleague Prabhat Sahu reported off list that transition tables
don't work for views. I probably should have thought about that when
I fixed something similar for partitioned tables, and after some
experimentation I see that this is also broken for foreign tables.
Good spot. Don't know why that didn't occur to me to look at.
For foreign tables using postgres_fdw, I see that transition tables
capture new rows for INSERT but capture nothing for DELETE and UPDATE.
Will dig into that in a bit, but first...
For views, aside from the question of transition tables, I noticed
that statement triggers don't seem to fire at all with updatable
views. Surely they should -- isn't that a separate bug?Example:
create table my_table (i int);
create view my_view as select * from my_table;
create function my_trigger_function() returns trigger language plpgsql as
$$ begin raise warning 'hello world'; return null; end; $$;
create trigger my_trigger after insert or update or delete on my_view
for each statement execute procedure my_trigger_function();
insert into my_view values (42);... and the world remains ungreeted.
I checked out 25dc142a (before any of my commits for $subject),
built it, and tried the above -- with no warning generated. I then
used an UPDATE and DELETE against the view, also with no trigger
fired (nor any error during trigger creation or DML). Does anyone
know whether such trigger ever fired at any point in the commit
history? Should we remove the documentation that anything except
INSTEAD OF triggers work on a view, and generate errors for attempt
to do otherwise, or is there something to salvage that has worked at
some point? If we do get these working, don't they deserve at least
one regression test?
Will post again after I have a chance to review the FDW issue.
--
Kevin Grittner
VMware vCenter Server
https://www.vmware.com/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Kevin Grittner <kgrittn@gmail.com> writes:
On Tue, Apr 25, 2017 at 6:17 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:For views, aside from the question of transition tables, I noticed
that statement triggers don't seem to fire at all with updatable
views. Surely they should -- isn't that a separate bug?
I checked out 25dc142a (before any of my commits for $subject),
built it, and tried the above -- with no warning generated. I then
used an UPDATE and DELETE against the view, also with no trigger
fired (nor any error during trigger creation or DML). Does anyone
know whether such trigger ever fired at any point in the commit
history?
[ experiments... ] They did, and do, fire if you do it the old-style
way with an INSTEAD OF row trigger. They don't fire if you're relying
on an updatable view. It seems we fire the table's statement triggers
instead, ie, the update is fully translated into an update on the
underlying table.
I'm not sure how intentional that was, but it's not a completely
unreasonable definition on its face, and given the lack of field
complaints since 9.3, I think we should probably stick to it.
However, if you didn't understand that from the documentation,
then we have a documentation problem.
If we do get these working, don't they deserve at least
one regression test?
Are you sure there isn't one?
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Apr 28, 2017 at 2:42 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Kevin Grittner <kgrittn@gmail.com> writes:
On Tue, Apr 25, 2017 at 6:17 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:For views, aside from the question of transition tables, I noticed
that statement triggers don't seem to fire at all with updatable
views. Surely they should -- isn't that a separate bug?I checked out 25dc142a (before any of my commits for $subject),
built it, and tried the above -- with no warning generated. I then
used an UPDATE and DELETE against the view, also with no trigger
fired (nor any error during trigger creation or DML). Does anyone
know whether such trigger ever fired at any point in the commit
history?[ experiments... ] They did, and do, fire if you do it the old-style
way with an INSTEAD OF row trigger.
Here is the table from near the start of CREATE TRIGGER docs,
"folded" such that I hope it remains intelligible in a fixed-width
font after email systems get done with it:
When
Event
Row-level Statement-level
BEFORE
INSERT/UPDATE/DELETE
Tables and foreign tables Tables, views, and foreign tables
TRUNCATE
— Tables
AFTER
INSERT/UPDATE/DELETE
Tables and foreign tables Tables, views, and foreign tables
TRUNCATE
— Tables
INSTEAD OF
INSERT/UPDATE/DELETE
Views —
TRUNCATE
— —
The claim seems to be that you can create a { BEFORE | AFTER } {
event [ OR ... ] } ... FOR EACH STATEMENT trigger where event is {
INSERT | UPDATE | DELETE } on an updateable view. Well, you can
*create* it, but it will never fire.
They don't fire if you're relying
on an updatable view. It seems we fire the table's statement triggers
instead, ie, the update is fully translated into an update on the
underlying table.
Well, certainly that should *also* happen. Not firing a table's DML
because it was fired off a view would be crazy, or so it seems to
me.
I'm not sure how intentional that was, but it's not a completely
unreasonable definition on its face, and given the lack of field
complaints since 9.3, I think we should probably stick to it.
Are you talking about being able to create INSERT, UPDATE, and
DELETE triggers on the view (which never fire), or about firing
triggers on the table when an INSTEAD OF trigger is fired.
However, if you didn't understand that from the documentation,
then we have a documentation problem.
I totally get that there are INSTEAD OF triggers and have no quibble
with how they behave. I can't even argue that the above chart is
wrong in terms of what CREATE TRIGGER allows. However, creating
triggers that can never fire seems entirely wrong.
If we do get these working, don't they deserve at least
one regression test?Are you sure there isn't one?
Well, I was sort of hoping that the triggers that can now be defined
but can never fire *did* fire at some point. But if that were true,
and they subsequently were broken, it would mean we lacked
regression tests for that case.
--
Kevin Grittner
VMware vCenter Server
https://www.vmware.com/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Apr 28, 2017 at 3:54 PM, Kevin Grittner <kgrittn@gmail.com> wrote:
Not firing a table's DML because it was fired off a view would be crazy,
should read:
Not firing a table's trigger because the trigger is on DML run from a
view's INSTEAD OF trigger would be crazy,
--
Kevin Grittner
VMware vCenter Server
https://www.vmware.com/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Kevin Grittner <kgrittn@gmail.com> writes:
Well, I was sort of hoping that the triggers that can now be defined
but can never fire *did* fire at some point.
They will fire if you have an INSTEAD OF row-level trigger; the existence
of that trigger is what determines whether we implement DML on a view
through the view's own triggers or through translation to an action on
the underlying table.
I do not think it'd be reasonable to throw an error for creation of
a statement-level view trigger when there's no row-level trigger,
because that just imposes a hard-to-deal-with DDL ordering dependency.
You could make a case for having the updatable-view translation code
print a WARNING if it notices that there are statement-level triggers
that cannot be fired due to the translation.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Apr 28, 2017 at 05:07:31AM +0000, Noah Misch wrote:
On Wed, Apr 26, 2017 at 11:17:05AM +1200, Thomas Munro wrote:
My colleague Prabhat Sahu reported off list that transition tables
don't work for views. I probably should have thought about that when
I fixed something similar for partitioned tables, and after some
experimentation I see that this is also broken for foreign tables.For foreign tables using postgres_fdw, I see that transition tables
capture new rows for INSERT but capture nothing for DELETE and UPDATE.For views, aside from the question of transition tables, I noticed
that statement triggers don't seem to fire at all with updatable
views. Surely they should -- isn't that a separate bug?Example:
create table my_table (i int);
create view my_view as select * from my_table;
create function my_trigger_function() returns trigger language plpgsql as
$$ begin raise warning 'hello world'; return null; end; $$;
create trigger my_trigger after insert or update or delete on my_view
for each statement execute procedure my_trigger_function();
insert into my_view values (42);... and the world remains ungreeted.
As for transition tables, there are probably meaningful ways to
support those for both views and foreign tables at least in certain
cases, as future feature enhancements. For now, do you agree that we
should reject such triggers as unsupported? See attached.[Action required within three days. This is a generic notification.]
The above-described topic is currently a PostgreSQL 10 open item. Kevin,
since you committed the patch believed to have created it, you own this open
item. If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know. Otherwise, please observe the policy on
open item ownership[1] and send a status update within three calendar days of
this message. Include a date for your subsequent status update. Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10. Consequently, I will appreciate your efforts
toward speedy resolution. Thanks.[1] /messages/by-id/20170404140717.GA2675809@tornado.leadboat.com
This PostgreSQL 10 open item is past due for your status update. Kindly send
a status update within 24 hours, and include a date for your subsequent status
update. Refer to the policy on open item ownership:
/messages/by-id/20170404140717.GA2675809@tornado.leadboat.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Apr 28, 2017 at 10:56 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
They will fire if you have an INSTEAD OF row-level trigger; the existence
of that trigger is what determines whether we implement DML on a view
through the view's own triggers or through translation to an action on
the underlying table.I do not think it'd be reasonable to throw an error for creation of
a statement-level view trigger when there's no row-level trigger,
because that just imposes a hard-to-deal-with DDL ordering dependency.You could make a case for having the updatable-view translation code
print a WARNING if it notices that there are statement-level triggers
that cannot be fired due to the translation.
Oh, I see -- you can add all the AFTER ... FOR EACH STATEMENT
triggers you want for an updatable view and they will quietly sit
there without firing no matter how many statements perform the
supposedly triggering action, but as soon as you add a INSTEAD OF
... FOR EACH ROW trigger they spring to life. On the face of it that
seems to me to violate the POLA, but I kinda see how it evolved.
I need to look at this and the rather similar odd behavior under
inheritance. I hope to post something Friday.
--
Kevin Grittner
VMware vCenter Server
https://www.vmware.com/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, May 02, 2017 at 06:06:47PM -0500, Kevin Grittner wrote:
On Fri, Apr 28, 2017 at 10:56 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
They will fire if you have an INSTEAD OF row-level trigger; the existence
of that trigger is what determines whether we implement DML on a view
through the view's own triggers or through translation to an action on
the underlying table.I do not think it'd be reasonable to throw an error for creation of
a statement-level view trigger when there's no row-level trigger,
because that just imposes a hard-to-deal-with DDL ordering dependency.You could make a case for having the updatable-view translation code
print a WARNING if it notices that there are statement-level triggers
that cannot be fired due to the translation.Oh, I see -- you can add all the AFTER ... FOR EACH STATEMENT
triggers you want for an updatable view and they will quietly sit
there without firing no matter how many statements perform the
supposedly triggering action, but as soon as you add a INSTEAD OF
... FOR EACH ROW trigger they spring to life. On the face of it that
seems to me to violate the POLA, but I kinda see how it evolved.I need to look at this and the rather similar odd behavior under
inheritance. I hope to post something Friday.
This PostgreSQL 10 open item is past due for your status update. Kindly send
a status update within 24 hours, and include a date for your subsequent status
update. Refer to the policy on open item ownership:
/messages/by-id/20170404140717.GA2675809@tornado.leadboat.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sat, May 06, 2017 at 06:58:35PM +0000, Noah Misch wrote:
On Tue, May 02, 2017 at 06:06:47PM -0500, Kevin Grittner wrote:
On Fri, Apr 28, 2017 at 10:56 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
They will fire if you have an INSTEAD OF row-level trigger; the existence
of that trigger is what determines whether we implement DML on a view
through the view's own triggers or through translation to an action on
the underlying table.I do not think it'd be reasonable to throw an error for creation of
a statement-level view trigger when there's no row-level trigger,
because that just imposes a hard-to-deal-with DDL ordering dependency.You could make a case for having the updatable-view translation code
print a WARNING if it notices that there are statement-level triggers
that cannot be fired due to the translation.Oh, I see -- you can add all the AFTER ... FOR EACH STATEMENT
triggers you want for an updatable view and they will quietly sit
there without firing no matter how many statements perform the
supposedly triggering action, but as soon as you add a INSTEAD OF
... FOR EACH ROW trigger they spring to life. On the face of it that
seems to me to violate the POLA, but I kinda see how it evolved.I need to look at this and the rather similar odd behavior under
inheritance. I hope to post something Friday.This PostgreSQL 10 open item is past due for your status update. Kindly send
a status update within 24 hours, and include a date for your subsequent status
update. Refer to the policy on open item ownership:
/messages/by-id/20170404140717.GA2675809@tornado.leadboat.com
IMMEDIATE ATTENTION REQUIRED. This PostgreSQL 10 open item is long past due
for your status update. Please reacquaint yourself with the policy on open
item ownership[1]/messages/by-id/20170404140717.GA2675809@tornado.leadboat.com and then reply immediately. If I do not hear from you by
2017-05-09 07:00 UTC, I will transfer this item to release management team
ownership without further notice.
[1]: /messages/by-id/20170404140717.GA2675809@tornado.leadboat.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, May 08, 2017 at 12:00:20AM -0700, Noah Misch wrote:
On Sat, May 06, 2017 at 06:58:35PM +0000, Noah Misch wrote:
On Tue, May 02, 2017 at 06:06:47PM -0500, Kevin Grittner wrote:
On Fri, Apr 28, 2017 at 10:56 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
They will fire if you have an INSTEAD OF row-level trigger; the existence
of that trigger is what determines whether we implement DML on a view
through the view's own triggers or through translation to an action on
the underlying table.I do not think it'd be reasonable to throw an error for creation of
a statement-level view trigger when there's no row-level trigger,
because that just imposes a hard-to-deal-with DDL ordering dependency.You could make a case for having the updatable-view translation code
print a WARNING if it notices that there are statement-level triggers
that cannot be fired due to the translation.Oh, I see -- you can add all the AFTER ... FOR EACH STATEMENT
triggers you want for an updatable view and they will quietly sit
there without firing no matter how many statements perform the
supposedly triggering action, but as soon as you add a INSTEAD OF
... FOR EACH ROW trigger they spring to life. On the face of it that
seems to me to violate the POLA, but I kinda see how it evolved.I need to look at this and the rather similar odd behavior under
inheritance. I hope to post something Friday.This PostgreSQL 10 open item is past due for your status update. Kindly send
a status update within 24 hours, and include a date for your subsequent status
update. Refer to the policy on open item ownership:
/messages/by-id/20170404140717.GA2675809@tornado.leadboat.comIMMEDIATE ATTENTION REQUIRED. This PostgreSQL 10 open item is long past due
for your status update. Please reacquaint yourself with the policy on open
item ownership[1] and then reply immediately. If I do not hear from you by
2017-05-09 07:00 UTC, I will transfer this item to release management team
ownership without further notice.[1] /messages/by-id/20170404140717.GA2675809@tornado.leadboat.com
This PostgreSQL 10 open item now needs a permanent owner. Would any other
committer like to take ownership? If this role interests you, please read
this thread and the policy linked above, then send an initial status update
bearing a date for your subsequent status update. If the item does not have a
permanent owner by 2017-05-13 03:00 UTC, I will resolve the item by reverting
transition table patches.
Thanks,
nm
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hi,
Since the last one conflicted with recent changes, here's a rebased
patch to prevent transition tables on views and foreign tables.
--
Thomas Munro
http://www.enterprisedb.com
Attachments:
prevent-unsupported-transition-tables-v2.patchapplication/octet-stream; name=prevent-unsupported-transition-tables-v2.patchDownload
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index d05e51c8208..c1ac5f93dbe 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -361,6 +361,20 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
RelationGetRelationName(rel)),
errdetail("Triggers on partitioned tables cannot have transition tables.")));
+ if (rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("\"%s\" is a foreign table",
+ RelationGetRelationName(rel)),
+ errdetail("Transition tables are not supported for triggers on foreign tables.")));
+
+ if (rel->rd_rel->relkind == RELKIND_VIEW)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("\"%s\" is a view",
+ RelationGetRelationName(rel)),
+ errdetail("Transition tables are not supported for triggers on views.")));
+
if (stmt->timing != TRIGGER_TYPE_AFTER)
ereport(ERROR,
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out
index af327d030d9..4e94df7667f 100644
--- a/src/test/regress/expected/foreign_data.out
+++ b/src/test/regress/expected/foreign_data.out
@@ -1269,6 +1269,13 @@ CREATE TRIGGER trigtest_after_stmt AFTER INSERT OR UPDATE OR DELETE
ON foreign_schema.foreign_table_1
FOR EACH STATEMENT
EXECUTE PROCEDURE dummy_trigger();
+CREATE TRIGGER trigtest_after_stmt_tt AFTER INSERT OR UPDATE OR DELETE -- ERROR
+ON foreign_schema.foreign_table_1
+REFERENCING NEW TABLE AS new_table
+FOR EACH STATEMENT
+EXECUTE PROCEDURE dummy_trigger();
+ERROR: "foreign_table_1" is a foreign table
+DETAIL: Transition tables are not supported for triggers on foreign tables.
CREATE TRIGGER trigtest_before_row BEFORE INSERT OR UPDATE OR DELETE
ON foreign_schema.foreign_table_1
FOR EACH ROW
diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out
index 10a301310b4..3827fde5478 100644
--- a/src/test/regress/expected/triggers.out
+++ b/src/test/regress/expected/triggers.out
@@ -1785,9 +1785,24 @@ drop trigger my_trigger on my_table_42;
create trigger my_trigger after update on my_table_42 referencing old table as old_table
for each statement execute procedure my_trigger_function();
drop trigger my_trigger on my_table_42;
+drop function my_trigger_function();
drop table my_table_42;
drop table my_table;
--
+-- Verify that triggers with transition tables are not allowed on
+-- views
+--
+create table my_table (i int);
+create view my_view as select * from my_table;
+create function my_trigger_function() returns trigger as $$ begin end; $$ language plpgsql;
+create trigger my_trigger after update on my_view referencing old table as old_table
+ for each statement execute procedure my_trigger_function();
+ERROR: "my_view" is a view
+DETAIL: Transition tables are not supported for triggers on views.
+drop function my_trigger_function();
+drop view my_view;
+drop table my_table;
+--
-- Verify that per-statement triggers are fired for partitioned tables
--
create table parted_stmt_trig (a int) partition by list (a);
diff --git a/src/test/regress/sql/foreign_data.sql b/src/test/regress/sql/foreign_data.sql
index ba528b5d36b..49255e309c1 100644
--- a/src/test/regress/sql/foreign_data.sql
+++ b/src/test/regress/sql/foreign_data.sql
@@ -527,6 +527,12 @@ ON foreign_schema.foreign_table_1
FOR EACH STATEMENT
EXECUTE PROCEDURE dummy_trigger();
+CREATE TRIGGER trigtest_after_stmt_tt AFTER INSERT OR UPDATE OR DELETE -- ERROR
+ON foreign_schema.foreign_table_1
+REFERENCING NEW TABLE AS new_table
+FOR EACH STATEMENT
+EXECUTE PROCEDURE dummy_trigger();
+
CREATE TRIGGER trigtest_before_row BEFORE INSERT OR UPDATE OR DELETE
ON foreign_schema.foreign_table_1
FOR EACH ROW
diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql
index 84b5ada5544..e5dbcaeea36 100644
--- a/src/test/regress/sql/triggers.sql
+++ b/src/test/regress/sql/triggers.sql
@@ -1261,10 +1261,25 @@ drop trigger my_trigger on my_table_42;
create trigger my_trigger after update on my_table_42 referencing old table as old_table
for each statement execute procedure my_trigger_function();
drop trigger my_trigger on my_table_42;
+drop function my_trigger_function();
drop table my_table_42;
drop table my_table;
--
+-- Verify that triggers with transition tables are not allowed on
+-- views
+--
+
+create table my_table (i int);
+create view my_view as select * from my_table;
+create function my_trigger_function() returns trigger as $$ begin end; $$ language plpgsql;
+create trigger my_trigger after update on my_view referencing old table as old_table
+ for each statement execute procedure my_trigger_function();
+drop function my_trigger_function();
+drop view my_view;
+drop table my_table;
+
+--
-- Verify that per-statement triggers are fired for partitioned tables
--
create table parted_stmt_trig (a int) partition by list (a);
On Tue, May 9, 2017 at 11:24 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
Since the last one conflicted with recent changes, here's a rebased
patch to prevent transition tables on views and foreign tables.
Committed, but I made the new error details more like the existing one
that also checks relkind.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers