Re: Command Triggers patch v18 - Mailing list pgsql-hackers

Robert Haas <robertmhaas@gmail.com> writes:
> 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...

What about :
 create command trigger foo before prepare alter table … create command trigger foo before start of alter table …
createcommand trigger foo before execute alter table … create command trigger foo before end of alter table … 
 create command trigger foo when prepare alter table … create command trigger foo when start alter table … create
commandtrigger foo when execute of alter table … create command trigger foo when end of alter table … 
 create command trigger foo preceding alter table … create command trigger foo preceding alter table … deferred create
commandtrigger foo preceding alter table … immediate 
 create command trigger foo following parser of alter table … create command trigger foo preceding execute of alter
table… 
 create command trigger foo following alter table …
 create command trigger foo before alter table nowait …

I'm not sure how many hooks we can really export here, but I guess we
have enough existing keywords to describe when they get to run (parser,
mapping, lock, check, begin, analyze, access, disable, not exists, do,
end, escape, extract, fetch, following, search…)

> 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

I've been trying to do that already.

> 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

That's why I implemented ALTER COMMAND TRIGGER ... SET DISABLE in the
first place, so that you could run the same command from the trigger
itself without infinite recursion.

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

I didn't think of DROP TABLE in a command table running BEFORE ALTER
TABLE, say. That looks evil. It could be documented as yet another way
to shoot yourself in the foot though?

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