On Tue, Mar 27, 2012 at 4:05 PM, Andres Freund <and...@2ndquadrant.com> wrote: >> > Looking up oids and such before calling the trigger doesn't seem to be >> > problematic from my pov. Command triggers are a superuser only facility, >> > additional information they gain are no problem. >> > Thinking about that I think we should enforce command trigger functions >> > to be security definer functions. >> I don't see any benefit from that restriction. > The reason I was thinking it might be a good idea is that they get might get > more knowledge passed than the user could get directly otherwise. Especially > if we extend the scheme to more places/informations.
As long as the superuser gets to decide whether or not a given function is installed as a command trigger, there's no harm in allowing him to make it either SECURITY INVOKER or SECURITY DEFINER. I agree that making it SECURITY DEFINER will often be useful and appropriate; I just see no reason to enforce that restriction programatically. > What are the phases we have: > > * after parse > * logging > * after resolving name > * after checking permisssions (interwined with the former) > * override permission check? INSTEAD? > * after locking (interwined with the former) > * except it needs to be connected to resolving the name/permission check > this doesn't seem to be an attractive hook point > * after validation > * additional validation likely want to hook here > * after execution > * replication might want to hook here > > Am I missing an interesting phase? I'd sort of categorize it like this: - pre-parse - post-parse - at permissions-checking time - post permissions-checking/name-lookup, before starting the main work of the command, i.e. pre-execution - "event" type triggers that happen when specific actions are performed (e.g. CREATE, DROP, etc.) or as subcommands fire (e.g. in ALTER TABLE, we could fire an alter-table-subcommand trigger every time we execute a subcommand) - post-execution > Allowing all that probably seems to require a substantial refactoring of > commands/ and catalog/ Probably. But we don't need to allow it all at once. What we need to do is pick one or two things that are relatively easily done, implement those first, and then build on it. Pre-parse, post-parse, CREATE-event, DROP-event, and post-execution hooks are all pretty easy to implement without major refactoring, I think. OTOH, post-permissions-checking-pre-execution is going to be hard to implement without refactoring, and ALTER-event hooks are going to be hard, too. > I think you need a surprisingly high amount of context to know when to ignore > a trigger. Especially as its not exactly easy to transfer knowledge from one > to the next. I'm not convinced, but maybe it would be easier to resolve this in the context of a specific proposal. > I don't think creating *new* DDL from the parsetree can really count as > statement based replication. And again, I don't think implementing that will > cost that much effort. > How would it help us to know - as individual events! - which tuples where > created where? Reassembling that will be a huge mess. I basically fail to see > *any* use case besides access checking. I think if you'd said this to me two years ago, I would have believed you, but poking through this code in the last year or two, I've come to the conclusion that there are a lot of subtle things that get worked out after parse time, during execution. Aside from things like recursing down the inheritance hierarchy and maybe creating some indexes or sequences when creating a table, there's also subtle things like working out exactly what index we're creating: name, access method, operator class, collation, etc. And I'm pretty sure that if we examine the code carefully we'll find there are a bunch more. > I fear a bit that this discussion is leading to something thats never going to > materialize because it would require a huge amount of work to get there. Again, the way to avoid that is to tackle the simple cases first and then work on the harder cases after that, but I don't think that's what the current patch is doing. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers