Re: Command Triggers, patch v11 - Mailing list pgsql-hackers

On 25 February 2012 16:36, Thom Brown <thom@linux.com> wrote:
> On 25 February 2012 14:30, Thom Brown <thom@linux.com> wrote:
>> On 25 February 2012 13:28, Thom Brown <thom@linux.com> wrote:
>>> On 25 February 2012 13:15, Thom Brown <thom@linux.com> wrote:
>>>> On 25 February 2012 12:42, Thom Brown <thom@linux.com> wrote:
>>>>> On 25 February 2012 12:07, Thom Brown <thom@linux.com> wrote:
>>>>>> On 25 February 2012 12:00, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote:
>>>>>>
>>>>>> D'oh, just as I sent some more queries...
>>>>>>
>>>>>>> Thom Brown <thom@linux.com> writes:
>>>>>>>> Is there any reason why the list of commands that command triggers can
>>>>>>>> be used with isn't in alphabetical order?  Also it appears to show
>>>>>>>
>>>>>>> Any reason why?  I don't suppose it's really important one way or the
>>>>>>> other, so I'm waiting on some more voices before working on it.
>>>>>>
>>>>>> Just so it's easy to scan.  If someone is looking for CREATE CAST,
>>>>>> they'd kind of expect it near the drop of the CREATE list, but it's
>>>>>> actually toward the bottom.  It just looks random at the moment.
>>>>>>
>>>>>>>> The ALTER COMMAND TRIGGER page also doesn't show which commands it can
>>>>>>>> be used against.  Perhaps, rather than repeat the list, there could be
>>>>>>>> a note to say that a list of valid commands can be found on the CREATE
>>>>>>>> COMMAND TRIGGER page?
>>>>>>>
>>>>>>> Well you can only alter a command that you were successful in creating,
>>>>>>> right?  So I'm not sure that's needed here.  By that count though, I
>>>>>>> maybe should remove the supported command list from DROP COMMAND TRIGGER
>>>>>>> reference page?
>>>>>>
>>>>>> Sure, that would be more consistent.  You're right, it's not needed.
>>>>>> It just seemed odd that one of the statements lacked what both others
>>>>>> had.
>>>>>
>>>>> Yet another comment... (I should have really started looking at this
>>>>> at an earlier stage)
>>>>>
>>>>> It seems that if one were to enforce a naming convention for relations
>>>>> as shown in the 2nd example for CREATE COMMAND TRIGGER, it could be
>>>>> circumvented by someone using CREATE TABLE name AS...
>>>>>
>>>>> test=# CREATE TABLE badname (id int, a int, b text);
>>>>> ERROR:  invalid relation name: badname
>>>>> test=# CREATE TABLE badname AS SELECT 1::int id, 1::int a, ''::text b;
>>>>> SELECT 1
>>>>>
>>>>> This doesn't even get picked up by ANY COMMAND.
>>>>
>>>> CREATE COMMAND TRIGGER doesn't output in pg_dump or pg_dumpall.  I'd
>>>> expect ALTER COMMAND TRIGGER to output too for when individual
>>>> commands are disabled etc.
>>>
>>> Just found another case where a table can be created without a command
>>> trigger firing:
>>>
>>> SELECT * INTO badname FROM goodname;
>>
>> Right, hopefully this should be my last piece of list spam for the
>> time being. (apologies, I thought I'd just try it out at first, but
>> it's ended up being reviewed piecemeal)
>
> I was wrong.. a couple of corrections to my own response:
>
>> On CREATE COMMAND TRIGGER page:
>>
>> “The trigger will be associated with the specified command and will
>> execute the specified function function_name when that command is
>> run.”
>> should be:
>> “The trigger will be associated with the specified commands and will
>> execute the specified function function_name when those commands are
>> run.”
>
> Actually, perhaps "...when any of those commands..."
>
>> On ALTER COMMAND TRIGGER page:
>>
>> “ALTER COMMAND TRIGGER name ON command SET enabled”
>> should be:
>> “ALTER COMMAND TRIGGER name ON command [, ... ] SET enabled”
>
> This one is nonsense, so please ignore it.

Further testing reveals a problem with FTS configurations when using
the example function provided in the docs:

test=# CREATE TEXT SEARCH CONFIGURATION test ( PARSER = "default"
);
ERROR:  invalid relation name:
test=# CREATE TEXT SEARCH CONFIGURATION fr_test ( PARSER = "default"
);
ERROR:  invalid relation name:

The 2nd one should work as it matches the naming convention checked in
the function.  The ALTER and DROP equivalents appear to be fine
though.

DROP CAST shares a similar issue too:

test=# DROP CAST (bigint as int4);
ERROR:  invalid relation name: �

The odd thing about this one is that CREATE CAST shouldn't match on
name at all, but it creates a cast successfully, whereas DROP CAST
disagrees with the name.

Command triggers for CREATE TYPE don't work, but fine for ALTER TYPE
and DROP TYPE.

Also command triggers for DROP CONVERSION aren't working.  A glance at
pg_cmdtrigger shows that the system views the command as "DROP
CONVERSION_P".

What is DROP ASSERTION?  It's showing as a valid command for a command
trigger, but it's not documented.

I've noticed that ALTER <object> name OWNER TO role doesn't result in
any trigger being fired except for tables.

ALTER OPERATOR FAMILY .... RENAME TO ... doesn't fire command triggers.

ALTER OPERATOR CLASS with RENAME TO or OWNER TO doesn't fire command
triggers, but with SET SCHEMA it does.

And there's no command trigger available for ALTER VIEW.

I'll hold off on testing any further until a new patch is available.

--
Thom