cannot set view triggers to replica
It appears to be an omission that ALTER TABLE ... ENABLE TRIGGER and
similar commands don't allow acting on views, even though we now have
triggers on views.
Similarly, the ALTER TABLE ... ENABLE RULE commands only allow acting on
tables, even though rules can also exist on views and materialized views.
(Why don't we allow rules on foreign tables? Is that intentional?)
Attached is a sample patch. It appears we don't have any regression
tests for this.
Attachments:
alter-table-enable-trigger-view.patchtext/x-diff; name=alter-table-enable-trigger-view.patchDownload
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 84dbee0..e530953 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -3341,13 +3341,18 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
case AT_DisableTrig: /* DISABLE TRIGGER variants */
case AT_DisableTrigAll:
case AT_DisableTrigUser:
- ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE);
+ ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE | ATT_VIEW);
pass = AT_PASS_MISC;
break;
case AT_EnableRule: /* ENABLE/DISABLE RULE variants */
case AT_EnableAlwaysRule:
case AT_EnableReplicaRule:
case AT_DisableRule:
+ ATSimplePermissions(rel, ATT_TABLE | ATT_MATVIEW | ATT_VIEW);
+ /* These commands never recurse */
+ /* No command-specific prep needed */
+ pass = AT_PASS_MISC;
+ break;
case AT_AddOf: /* OF */
case AT_DropOf: /* NOT OF */
case AT_EnableRowSecurity:
On Sat, May 30, 2015 at 11:47 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
It appears to be an omission that ALTER TABLE ... ENABLE TRIGGER and
similar commands don't allow acting on views, even though we now have
triggers on views.
True, now isn't it something that should be as well part of ALTER VIEW?
Similarly, the ALTER TABLE ... ENABLE RULE commands only allow acting on
tables, even though rules can also exist on views and materialized views.
I think that ALTER VIEW and ALTER MATERIALIZED VIEW should be able to
accept the command as well.
Attached is a sample patch. It appears we don't have any regression
tests for this.
This sounds like a mandatory condition for this patch.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 30 May 2015 at 03:47, Peter Eisentraut <peter_e@gmx.net> wrote:
It appears to be an omission that ALTER TABLE ... ENABLE TRIGGER and
similar commands don't allow acting on views, even though we now have
triggers on views.
It was deliberately omitted from the original INSTEAD OF triggers
patch because it didn't seem that useful (and to keep the patch size
down), see:
/messages/by-id/AANLkTimJw47yZHnxKhMNLCFES=W-sMrqpRe7aj8YBKds@mail.gmail.com
That said, I have no objection to allowing it if you think it's likely
to be useful. I think you will need to work a little harder to make it
work properly though. Consider, for example, how
view_has_instead_trigger() and its callers, and CheckValidResultRel()
should behave if the triggers are disabled.
Regards,
Dean
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Michael Paquier <michael.paquier@gmail.com> wrote:> On Sat, May 30, 2015 at 11:47 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
the ALTER TABLE ... ENABLE RULE commands only allow acting on>> tables, even though rules can also exist on views and materialized views.
I think that ALTER VIEW and ALTER MATERIALIZED VIEW should be able to
accept the command as well.
What would be a use case for disabling the _RETURN rule for a [ MATERIALIZED ] VIEW?
--
Kevin Grittner
EDB: 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