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-25T16:36:41Z
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 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.

-- 
Thom