Re: Command Triggers

Hi,

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> That seems icky. Whatever warnings need to be given should be in the
> documentation, not at runtime.

So, new patch is attached, with some more docs, but it's still shy on it
though, mainly because I don't have a setup to build docs easily here.
I intend to be completing docs on Monday.

If you agree with the way it's implemented now, I'll integrate some more
commands (all our commands expect for those working with shared objects,
and restricting away after command trigger support for commands that
handle transactions themselves).

Once those two items are done, I believe we will reach the Grail^W
“ready for commit” status, and that should happen early next week.

>>  CREATE COMMAND TRIGGER
>>  DROP COMMAND TRIGGER
>>  ALTER COMMAND TRIGGER
>
> 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.

This is implementing in the attached patch, v10.

>> 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. :-)

So now I've integrated with RemoveObjects(), which means it's no longer
possible to drop a command trigger attached to more than one command in
one go. Not a show stopper, but maybe worth noting here.

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

I'm still not convinced that two index scans will be noticeable when
executing a DDL command. So it all feels like premature optimization to
me, even if I completely understand where you come from (most of the
patch you worked on for this release share an easy to spot common theme
after all).

So I didn't implement a cache here yet in patch v10. I would hate to
spend time on that to chase a 1% DDL-only performance regression.

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support