Re: Command Triggers, patch v11

On 6 March 2012 23:25, Thom Brown <thom(at)linux(dot)com> wrote:
> On 6 March 2012 21:18, Thom Brown <thom(at)linux(dot)com> wrote:
>> On 6 March 2012 21:04, Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr> wrote:
>>>>> [CASCADE will not run the command triggers for cascaded objects]
>>>> If these are all expected, does it in any way compromise the
>>>> effectiveness of DDL triggers in major use-cases?
>>>
>>> I don't think so.  When replicating the replica will certainly drop the
>>> same set of dependent objects, for example.  Auditing is another story.
>>> Do we want to try having cascaded object support in drop commands?
>>
>> I wasn't sure if auditing was one of the rationale behind the feature
>> or not.  If it is, it presents a major problem.  How does the replica
>> know that the objects were dropped?
>>
>> Thanks for the updated patch and the quick turnaround time.  I'll give
>> it another review.
>
> Okay, here's the update:
>
> The message returned by creating a command trigger after create index
> is still problematic:
>
> thom(at)test=# CREATE COMMAND TRIGGER cmd_trg_after_create_index AFTER
> CREATE INDEX EXECUTE PROCEDURE cmd_trg_info();
> WARNING:  AFTER CREATE INDEX CONCURRENTLY triggers are not supported
> DETAIL:  The command trigger will not get fired.
> CREATE COMMAND TRIGGER
>
> The detail suggests that even though the command trigger has been
> requested, it won't be fired.  Might I suggest:
>
> WARNING:  AFTER CREATE INDEX CONCURRENTLY triggers are not supported
> DETAIL:  The command trigger will not fire on concurrently-created indexes.
>
>
>
> CREATE VIEW doesn't return schema:
>
> thom(at)test=# CREATE VIEW view_test AS SELECT id, stuff FROM public.test;
> NOTICE:  Command trigger on any: tg_when='BEFORE' cmd_tag='CREATE
> VIEW' objectid=<NULL> schemaname='<NULL>' objectname='<NULL>'
> NOTICE:  Command trigger: tg_when='BEFORE' cmd_tag='CREATE VIEW'
> objectid=<NULL> schemaname='<NULL>' objectname='view_test'
> NOTICE:  Command trigger: tg_when='AFTER' cmd_tag='CREATE VIEW'
> objectid=16606 schemaname='<NULL>' objectname='view_test'
> NOTICE:  Command trigger on any: tg_when='AFTER' cmd_tag='CREATE VIEW'
> objectid=<NULL> schemaname='<NULL>' objectname='<NULL>'
> CREATE VIEW
>
> No specific triggers fire when altering a conversion:
>
> thom(at)test=# ALTER CONVERSION test9 RENAME TO test8;
> NOTICE:  Command trigger on any: tg_when='BEFORE' cmd_tag='ALTER
> CONVERSION' objectid=<NULL> schemaname='<NULL>' objectname='<NULL>'
> NOTICE:  Command trigger on any: tg_when='AFTER' cmd_tag='ALTER
> CONVERSION' objectid=<NULL> schemaname='<NULL>' objectname='<NULL>'
> ALTER CONVERSION
>
> Note: I hadn’t previously tested this, but I don’t think it was listed
> as supported until now.
>
>
> No specific triggers fire when altering the properties of a function:
>
> thom(at)test=# ALTER FUNCTION test.testfunc2() COST 77;
> NOTICE:  Command trigger on any: tg_when='BEFORE' cmd_tag='ALTER
> FUNCTION' objectid=<NULL> schemaname='<NULL>' objectname='<NULL>'
> NOTICE:  Command trigger on any: tg_when='AFTER' cmd_tag='ALTER
> FUNCTION' objectid=<NULL> schemaname='<NULL>' objectname='<NULL>'
> ALTER FUNCTION
>
>
> No specific triggers fire when altering a sequence:
>
> thom(at)test=# ALTER SEQUENCE test_seq2 OWNER TO test;
> NOTICE:  Command trigger on any: tg_when='BEFORE' cmd_tag='ALTER
> SEQUENCE' objectid=<NULL> schemaname='<NULL>' objectname='<NULL>'
> NOTICE:  Command trigger on any: tg_when='AFTER' cmd_tag='ALTER
> SEQUENCE' objectid=<NULL> schemaname='<NULL>' objectname='<NULL>'
> ALTER SEQUENCE
>
> thom(at)test=# ALTER SEQUENCE test_seq2 RESTART WITH 4;
> NOTICE:  Command trigger on any: tg_when='BEFORE' cmd_tag='ALTER
> SEQUENCE' objectid=<NULL> schemaname='<NULL>' objectname='<NULL>'
> NOTICE:  Command trigger on any: tg_when='AFTER' cmd_tag='ALTER
> SEQUENCE' objectid=<NULL> schemaname='<NULL>' objectname='<NULL>'
> ALTER SEQUENCE
>
>
> No specific triggers when altering a view:
>
> thom(at)test=# ALTER VIEW view_test2 SET SCHEMA testnew;
> NOTICE:  Command trigger on any: tg_when='BEFORE' cmd_tag='ALTER VIEW'
> objectid=<NULL> schemaname='<NULL>' objectname='<NULL>'
> NOTICE:  Command trigger on any: tg_when='AFTER' cmd_tag='ALTER VIEW'
> objectid=<NULL> schemaname='<NULL>' objectname='<NULL>'
> ALTER VIEW
>
> thom(at)test=# ALTER VIEW testnew.view_test2 ALTER COLUMN id SET DEFAULT 9;
> NOTICE:  Command trigger on any: tg_when='BEFORE' cmd_tag='ALTER VIEW'
> objectid=<NULL> schemaname='<NULL>' objectname='<NULL>'
> NOTICE:  Command trigger on any: tg_when='AFTER' cmd_tag='ALTER VIEW'
> objectid=<NULL> schemaname='<NULL>' objectname='<NULL>'
> ALTER VIEW
>
>
> The object name shown in specific triggers when dropping aggregates
> shows the schema name:
>
> thom(at)test=# DROP AGGREGATE testnew.avgtest2(bigint);
> NOTICE:  Command trigger on any: tg_when='BEFORE' cmd_tag='DROP
> AGGREGATE' objectid=<NULL> schemaname='<NULL>' objectname='<NULL>'
> NOTICE:  Command trigger: tg_when='BEFORE' cmd_tag='DROP AGGREGATE'
> objectid=16539 schemaname='testnew' objectname='testnew'
> NOTICE:  Command trigger: tg_when='AFTER' cmd_tag='DROP AGGREGATE'
> objectid=<NULL> schemaname='testnew' objectname='testnew'
> NOTICE:  Command trigger on any: tg_when='AFTER' cmd_tag='DROP
> AGGREGATE' objectid=<NULL> schemaname='<NULL>' objectname='<NULL>'
> DROP AGGREGATE
>
> Same for collations:
>
> thom(at)test=# DROP COLLATION testnew.en_gb_test;
> NOTICE:  Command trigger on any: tg_when='BEFORE' cmd_tag='DROP
> COLLATION' objectid=<NULL> schemaname='<NULL>' objectname='<NULL>'
> NOTICE:  Command trigger: tg_when='BEFORE' cmd_tag='DROP COLLATION'
> objectid=16542 schemaname='testnew' objectname='testnew'
> NOTICE:  Command trigger: tg_when='AFTER' cmd_tag='DROP COLLATION'
> objectid=<NULL> schemaname='testnew' objectname='testnew'
> NOTICE:  Command trigger on any: tg_when='AFTER' cmd_tag='DROP
> COLLATION' objectid=<NULL> schemaname='<NULL>' objectname='<NULL>'
> DROP COLLATION
>
>
> When dropping domains, the name of the domain includes the schema name:
>
> thom(at)test=# DROP DOMAIN testnew.us_postal_code;
> NOTICE:  Command trigger on any: tg_when='BEFORE' cmd_tag='DROP
> DOMAIN' objectid=<NULL> schemaname='<NULL>' objectname='<NULL>'
> NOTICE:  Command trigger: tg_when='BEFORE' cmd_tag='DROP DOMAIN'
> objectid=16546 schemaname='testnew'
> objectname='testnew.us_postal_code'
> NOTICE:  Command trigger: tg_when='AFTER' cmd_tag='DROP DOMAIN'
> objectid=<NULL> schemaname='testnew'
> objectname='testnew.us_postal_code'
> NOTICE:  Command trigger on any: tg_when='AFTER' cmd_tag='DROP DOMAIN'
> objectid=<NULL> schemaname='<NULL>' objectname='<NULL>'
> DROP DOMAIN
>
>
> Dropping functions shows the object name as the schema name:
>
> thom(at)test=# DROP FUNCTION testnew.testfunc2();
> NOTICE:  Command trigger on any: tg_when='BEFORE' cmd_tag='DROP
> FUNCTION' objectid=<NULL> schemaname='<NULL>' objectname='<NULL>'
> NOTICE:  Command trigger: tg_when='BEFORE' cmd_tag='DROP FUNCTION'
> objectid=16557 schemaname='testnew' objectname='testnew'
> NOTICE:  Command trigger: tg_when='AFTER' cmd_tag='DROP FUNCTION'
> objectid=<NULL> schemaname='testnew' objectname='testnew'
> NOTICE:  Command trigger on any: tg_when='AFTER' cmd_tag='DROP
> FUNCTION' objectid=<NULL> schemaname='<NULL>' objectname='<NULL>'
> DROP FUNCTION
>
>
> Same with dropping operators:
>
> thom(at)test=# DROP OPERATOR testnew.<<|| (int2, int2);
> NOTICE:  Command trigger on any: tg_when='BEFORE' cmd_tag='DROP
> OPERATOR' objectid=<NULL> schemaname='<NULL>' objectname='<NULL>'
> NOTICE:  Command trigger: tg_when='BEFORE' cmd_tag='DROP OPERATOR'
> objectid=16566 schemaname='testnew' objectname='testnew'
> NOTICE:  Command trigger: tg_when='AFTER' cmd_tag='DROP OPERATOR'
> objectid=<NULL> schemaname='testnew' objectname='testnew'
> NOTICE:  Command trigger on any: tg_when='AFTER' cmd_tag='DROP
> OPERATOR' objectid=<NULL> schemaname='<NULL>' objectname='<NULL>'
> DROP OPERATOR
>
>
> ...and operator family:
>
> thom(at)test=# DROP OPERATOR FAMILY testnew.test_ops2 USING btree;
> NOTICE:  Command trigger on any: tg_when='BEFORE' cmd_tag='DROP
> OPERATOR FAMILY' objectid=<NULL> schemaname='<NULL>'
> objectname='<NULL>'
> NOTICE:  Command trigger: tg_when='BEFORE' cmd_tag='DROP OPERATOR
> FAMILY' objectid=16571 schemaname='testnew' objectname='testnew'
> NOTICE:  Command trigger: tg_when='AFTER' cmd_tag='DROP OPERATOR
> FAMILY' objectid=<NULL> schemaname='testnew' objectname='testnew'
> NOTICE:  Command trigger on any: tg_when='AFTER' cmd_tag='DROP
> OPERATOR FAMILY' objectid=<NULL> schemaname='<NULL>'
> objectname='<NULL>'
> DROP OPERATOR FAMILY
>
>
> … and the same for dropping text search
> configuration/dictionary/parser/template.
>
>
> I hadn't previously tested triggers against CLUSTER, REINDEX, VACUUM
> and LOAD, but have tested them now.
>
> When creating a trigger on REINDEX, I get the following message:
>
> thom(at)test=# CREATE COMMAND TRIGGER cmd_trg_before_reindex BEFORE
> REINDEX EXECUTE PROCEDURE cmd_trg_info();
> WARNING:  REINDEX DATABASE is not supported
> DETAIL:  The command trigger will not get fired.
>
> My problem with this is the same as what I reported previously for
> CREATE INDEX, in that it suggests the DDL itself isn't supported.
>
> But more importantly:
>
> Creating AFTER CLUSTER command triggers produce an error (as expected
> since it's not supported), but AFTER REINDEX only produces a warning.
> These should be the same, probably both an error.
>
>
> VACUUM doesn't fire a specific command trigger:
>
> thom(at)test=# VACUUM;
> NOTICE:  Command trigger on any: tg_when='BEFORE' cmd_tag='VACUUM'
> objectid=<NULL> schemaname='<NULL>' objectname='<NULL>'
> VACUUM
>
>
> REINDEX on a table seems to show no schema name but an object name for
> specific triggers:
>
> thom(at)test=# REINDEX TABLE testnew.test9;
> NOTICE:  Command trigger on any: tg_when='BEFORE' cmd_tag='REINDEX'
> objectid=<NULL> schemaname='<NULL>' objectname='<NULL>'
> NOTICE:  Command trigger: tg_when='BEFORE' cmd_tag='REINDEX'
> objectid=16558 schemaname='<NULL>' objectname='testnew'
> NOTICE:  Command trigger: tg_when='AFTER' cmd_tag='REINDEX'
> objectid=16558 schemaname='<NULL>' objectname='testnew'
> NOTICE:  Command trigger on any: tg_when='AFTER' cmd_tag='REINDEX'
> objectid=<NULL> schemaname='<NULL>' objectname='<NULL>'
> REINDEX
>
>
> When REINDEXing an index rather than a table, the table's details are
> shown in the trigger.  Is this expected?:
>
> thom(at)test=# REINDEX INDEX testnew.idx_test_2 ;
> NOTICE:  Command trigger on any: tg_when='BEFORE' cmd_tag='REINDEX'
> objectid=<NULL> schemaname='<NULL>' objectname='<NULL>'
> NOTICE:  Command trigger: tg_when='BEFORE' cmd_tag='REINDEX'
> objectid=16565 schemaname='testnew' objectname='test9'
> NOTICE:  Command trigger: tg_when='AFTER' cmd_tag='REINDEX'
> objectid=16565 schemaname='testnew' objectname='test9'
> NOTICE:  Command trigger on any: tg_when='AFTER' cmd_tag='REINDEX'
> objectid=<NULL> schemaname='<NULL>' objectname='<NULL>'
> REINDEX
>
>
> REINDEXing the whole database doesn't fire specific command triggers:
>
> thom(at)test=# REINDEX DATABASE test;
> NOTICE:  Command trigger on any: tg_when='BEFORE' cmd_tag='REINDEX'
> objectid=<NULL> schemaname='<NULL>' objectname='<NULL>'
> NOTICE:  table "pg_catalog.pg_class" was reindexed
> <snip>
> NOTICE:  table "information_schema.sql_features" was reindexed
> NOTICE:  Command trigger on any: tg_when='AFTER' cmd_tag='REINDEX'
> objectid=<NULL> schemaname='<NULL>' objectname='<NULL>'
> REINDEX
>
> The same happens for the REINDEX SYSTEM syntax.
>
>
> Documentation:
>
> I previously noted text which needed correcting that doesn’t seem to
> have fixed yet.  The email I reported those in misleadingly began with
> “Right, hopefully this should be my last piece of list spam...”.  The
> only one which I feel no longer needs fixing is the first thing I
> reported due to the fact that command triggers can now only been
> applied against one command at a time.  Also disregard the comments in
> that email for ALTER and DROP.
>
> You appear to have added support for ALTER CAST, but there isn’t any such DDL.
>
> OPERATOR CLASS and OPERATOR FAMILY are listed twice.
>
> “Note that objects dropped by effect of DROP CASCADE will not provoque
> firing a command trigger”
> should probably be:
> “Note that objects dropped by the effect of DROP CASCADE will not
> result in a command trigger firing.”
>
> “That also applis to other dependencies following, as in DROP OWNED BY.”
> should be:
> “That also applies to other dependencies following, as in DROP OWNED BY.”
>
> Another typo (which was in the email I referred to earlier) is
> s/exercize/exercise/.
>
> “Triggers on ANY command support more commands than just this list,
> and will only provide the command tag argument as NOT NULL.”
> should be:
> “Triggers on ANY command support more commands than just this list,
> and will provide NULL values for every argument except for the
> argument that determines whether the trigger was before or after the
> command event, and the command tag.”
>
> This will no doubt need changing to refer to the new trigger variables
> if you introduce them.

I've also since found that if I issue a VACUUM, CLUSTER or REINDEX on
a read-only standby, the BEFORE ANY COMMAND trigger fires. I don't
think any trigger should fire on a read-only standby.

--
Thom