Re: Command Triggers patch v18

On Wed, Mar 28, 2012 at 7:16 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> Now I can see why implementing that on top of the ANY command feature is
>> surprising enough for wanting to not do it this way. Maybe the answer is
>> to use another keyword to be able to register command triggers that run
>> that early and without any specific information other than the command
>> tag.
>
> Yeah, I think so.  I objected to the way you had it because of the
> inconsistency, not because I think it's a useless place to fire
> triggers.

Further thought: I think maybe we shouldn't use keywords at all for
this, and instead use descriptive strings like post-parse or
pre-execution or command-start, because I bet in the end we're going
to end up with a bunch of them (create-schema-subcommand-start?
alter-table-subcommand-start?), and it would be nice not to hassle
with the grammar every time we want to add a new one. To make this
work with the grammar, we could either (1) enforce that each is
exactly one word or (2) require them to be quoted or (3) require those
that are not a single word to be quoted. I tend think #2 might be the
best option in this case, but...

I've also had another general thought about safety: we need to make
sure that we're only firing triggers from places where it's safe for
user code to perform arbitrary actions. In particular, we have to
assume that any triggers we invoke will AcceptInvalidationMessages()
and CommandCounterIncrement(); and we probably can't do it at all (or
at least not without some additional safeguard) in places where the
code does CheckTableNotInUse() up through the point where it's once
again safe to access the table, since the trigger might try. We also
need to think about re-entrancy: that is, in theory, the trigger might
execute any other DDL command - e.g. it might drop the table that
we've been asked to rename. I suspect that some of the current BEFORE
trigger stuff might fall down on that last one, since the existing
code not-unreasonably expects that once it's locked the table, the
catalog entries can't go away. Maybe it all happens to work out OK,
but I don't think we can count on that.

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