Re: Command Triggers

On Fri, Feb 17, 2012 at 10:42 AM, Dimitri Fontaine
<dimitri(at)2ndquadrant(dot)fr> wrote:
> Done.  Of course at the time the command trigger is created you can't
> distinguish if the CREATE INDEX command will be run CONCURRENTLY or not,
> so I've decided to issue a WARNING about it.

That seems icky. Whatever warnings need to be given should be in the
documentation, not at runtime.

Another idea here would be to treat CREATE INDEX CONCURRENTLY as if it
were a separate toplevel command, for command-trigger purposes only.
But I'm not sure that's any better.

> You would need to set a command trigger on ALTER COMMAND TRIGGER and
> that's not supported. Triggers on command "ALTER TRIGGER" in fact will
> not get fired on ALTER TRIGGER ... ON COMMAND ...
>
> I guess that's a point to change the grammar the way you're hinting:

Indeed it is. :-)

>  CREATE COMMAND TRIGGER
>  DROP COMMAND TRIGGER
>  ALTER COMMAND TRIGGER
>
> That also needs each their own reference page.  It will be easier on the
> users I guess.  Will work on that.

Yeah, I think that will be much more clear, and not really that much
work for you. It will also make the reference pages simpler, I think,
since there are significant behavioral differences between ordinary
triggers and command triggers.

> Both done, if you agree with using session_replication_role here.

It's better than a sharp stick in the eye. I'm not convinced it's
ideal, but I don't feel strongly enough about the issue to push on it
for now, as long as we disallow command triggers on CREATE/ALTER/DROP
COMMAND TRIGGER.

>> Sure, but if you run the same trigger on multiple operations - INSERT
>> OR UPDATE OR DELETE.
>
> I failed to see that analogy. The other problem with the current way of
> doing things is that I can't integrate with RemoveObjects(), and I think
> you won't like that :)

I sure won't. I think ultimately you won't like it either, since the
objectaddress infrastructure is also needed to make this work with
extensions. And I assume you would agree with me that extensions are
an important feature. :-)

>>> You create a trigger on ANY command :)
>>
>> Oh.  Well, then +1 for me on the ANY COMMAND thing, but -1 on ALL
>> COMMANDS.  I can't see that there's enough utility to having a
>> bulk-create functionality to justify its existence.  The ANY COMMAND
>> thing I think is what people will want.
>
> There's no such thing as ALL COMMANDS in the patch, there's a syntactic
> sugar allowing you to create and drop more than one command trigger in a
> single command, much as we have DROP TABLE foo, bar, baz;

OK, I'll look more carefully.

>> I am thinking that we should ditch the idea of keeping track of
>> commands using strings and instead assign a bunch of integer constants
>> using a big enum.  The parser can translate from what the user enters
>> to these constants and then use those throughout, including in the
>> system catalogs.
>
> It's not really command strings but the Command Tag we've historically
> been using up until now.  You're saying that it should remain the same
> for users but change internally.  No strong opinion from me here, apart
> from it being more code for doing the same thing.

Well, the reason I thought it might be better is for caching purposes.
If you have a cache of which triggers need to be run for which
commands, an integer index into an array will be a lot faster than a
hash table lookup. But it may bear more examination, so I don't feel
this is a must-do at this point.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company