Hi, Please find an update attached, v4, fixing most remaining items. Next steps are better docs and more commands support (along with finishing currently supported ones), and a review locking behavior.
If you want to just scroll over the patch to get an impression of what's in there rather than try out the attachment, follow this URL: https://github.com/dimitri/postgres/compare/master...command_triggers Dimitri Fontaine <dimi...@2ndquadrant.fr> writes: > Will look into qualifying names. I'm now qualifying relation names even if they have not been entered with a namespace qualifier. What do you think? The other components are left alone, I think the internal APIs for qualifying all kind of objects from the parse tree and current context are mostly missing. >> As an alternative it would be possible to pass the current search path but >> that doesn't seem to the best approach to me; > > The trigger runs with the same search_path settings as the command, right? Maybe that's good enough for command triggers? >> Command triggers should only be allowed for the database owner. > > Yes, that needs to happen soon, along with pg_dump and psql support. All three are implemented in the attached new revision of the patch. >> Imo the current callsite in ProcessUtility isn't that good. I think it would >> make more sense moving it into standard_ProcessUtility. It should be *after* >> the check_xact_readonly check in there in my opinion. > > Makes sense, will fix in the next revision. Done too. It's better this way, thank you. >> I am also don't think its a good idea to run the >> ExecBeforeOrInsteadOfCommandTriggers stuff there because thats allso the >> path >> that COMMIT/BEGIN and other stuff take. Those are pretty "fast path". >> >> I suggest making two switches in standard_ProcessUtility, one for the non- >> command trigger stuff which returns on success and one after that. Only >> after >> the first triggers are checked. > > Agreed. That's about the way I've done it. Please note that doing it this way means that a ProcessUtility_hook can decide whether or not the command triggers are going to be fired or not, and that's true for BEFORE, AFTER and INSTEAD OF command triggers. I think that's the way to go, though. >> * Either tgenable's datatype or its readfunc is wrong (watch the compiler ;)) >> * ruleutils.c: >> * generic routine for IF EXISTS, CASCADE, ... > > Will have to wait until next revision. Fixed. Well, the generic routine would only be called twice and would only share a rather short expression, so will have to wait until I add support for more commands. There's a regression tests gotcha. Namely that the parallel running of triggers against inheritance makes it impossible to predict if the trigger on the command CREATE TABLE will spit out a notice in the inherit tests. I don't know how that is usually avoided, but I guess it involves picking some other command example that don't conflict with the parallel tests of that section? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
command-trigger.v4.patch.gz
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
