Re: Command Triggers, patch v11

Thom Brown <thom@linux.com>

From: Thom Brown <thom@linux.com>
To: Dimitri Fontaine <dimitri@2ndquadrant.fr>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2012-02-25T14:30:26Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Fix SPGiST vacuum algorithm to handle concurrent tuple motion properly.

  2. Remove useless const qualifier

  3. Make executor's SELECT INTO code save and restore original tuple receiver.

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)

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.”

“A command trigger's function must return void, the only it can aborts
the execution of the command is by raising an exception.”
should be:
“A command trigger's function must return void.  It can then only
abort the execution of the command by raising an exception.”

Remove:
“For a constraint trigger, this is also the name to use when modifying
the trigger's behavior using SET CONSTRAINTS.”

Remove:
“That leaves out the following list of non supported commands.”

s/exercize/exercise/

“that's the case for VACUUM, CLUSTER CREATE INDEX CONCURRENTLY, and
REINDEX DATABASE.”
should be:
“that's the case for VACUUM, CLUSTER, CREATE INDEX CONCURRENTLY, and
REINDEX DATABASE.”

I don’t understand this sentence:
“Triggers on ANY command support more commands than just this list,
and will only provide the command tag argument as NOT NULL.”


On ALTER COMMAND TRIGGER page:

“ALTER COMMAND TRIGGER name ON command SET enabled”
should be:
“ALTER COMMAND TRIGGER name ON command [, ... ] SET enabled”


On DROP COMMAND TRIGGER page:

There’s a mention of CASCADE and RESTRICT.  I don’t know of any object
which could be dependant on a command trigger, so I don’t see what
these are for.


An oddity I’ve noticed is that you can add additional commands to an
existing command trigger, and you can also have them execute a
different function to the other commands referenced in the same
trigger.

-- 
Thom