On 5 March 2012 20:42, Dimitri Fontaine <dimi...@2ndquadrant.fr> wrote: > Hi, > > Thanks for the extensive testing. I'm adding your tests to the > regression suite, and keep wondering if you saw that lots of them were > already covered? Did you try make installcheck?
Yes, but I felt it better that I come up with my own separate tests. > Thom Brown <t...@linux.com> writes: >> Creating a command trigger using ANY COMMAND results in oid, >> schemaname, objectname (function parameters 4 & 5) not being set for >> either BEFORE or AFTER. > > Yes, that's documented. It could be better documented though, it seems. Is there a reason why we can't provide the OID for ANY COMMAND... if it's available? I'm guessing it would end up involving having to special case for every command. :/ >> Command triggers for creating sequences don’t show the schema: > > Documented already, it's uneasy to get at it in the code and I figured I > might as well drop the ball on that in the current patch's form. Fair enough. >> Command triggers for BEFORE CREATE TYPE (exluding ANY COMMAND) don’t >> fire if the type isn’t created due to an error: > > Per design, although we might want to talk about it. I made it so that > specific command triggers are only fired after errors checks have been > made. > > That's not the case with ANY command triggers so that you can actually > block DDLs on your instances, as has been asked on list. I don't have any strong feelings about it, so I'll bear it in mind for future tests. >> The ANY COMMAND trigger fires on creating roles, but there’s no >> corresponding allowance to create the trigger explicitly for creating >> roles. > > Roles are global objects, you don't want the behavior of role commands > to depend on which database you happen to have been logged in when > issuing the command. That would call for removing the ANY command > support for them too, but I can't seem to decide about that. > > Any input? If that's your reasoning, then it would make sense to remove ANY command support for it too. >> There doesn’t appear to be command trigger support for ALTER LARGE OBJECT. > > Do we want to have some? Those are in between data and command. *shrug* But ANY COMMAND triggers fire for it. So I'd say either remove support for that, or add a specific trigger. >> Specific command triggers on DROP AGGREGATE don’t fire in the IF >> EXISTS scenario if the target object doesn’t exist: > > So, do we want to run the command triggers here? Is the IF EXISTS check > to be considered like the other error conditions? Maybe. If that's expected behaviour, I'll start expecting it then. >> When adding objects to an extension, then dropping the extension with >> a cascade, the objects are dropped with it, but triggers aren’t fired >> to the removal of those dependant objects: > > Yes, that's expected and needs documenting. > >> Using DROP OWNED BY allows objects to be dropped without their >> respective specific triggers firing. > > Expected too. > >> Using DROP SCHEMA … CASACDE also allows objects to be dropped without >> their respective specific triggers firing: > > Again, same expectation here. If these are all expected, does it in any way compromise the effectiveness of DDL triggers in major use-cases? > I'm not sending a revised patch, please use the github branch if you > want to do some more tests already, or ask me for either a new patch > version or a patch-on-patch, as you see fit. Hmm... how does that work with regards to the commitfest process? But I'll re-test when you let me know when you've committed your remaining fixes to Github. -- Thom -- Sent via pgsql-hackers mailing list (firstname.lastname@example.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers