Re: Command Triggers patch v18 - Mailing list pgsql-hackers
From | Robert Haas |
---|---|
Subject | Re: Command Triggers patch v18 |
Date | |
Msg-id | CA+TgmoYW6YP+8DxCg8DXgozY71n_-7DEmUsBqLbrxVns=x=yzw@mail.gmail.com Whole thread Raw |
In response to | Re: Command Triggers patch v18 (Thom Brown <thom@linux.com>) |
Responses |
Re: Command Triggers patch v18
|
List | pgsql-hackers |
On Thu, Mar 29, 2012 at 11:49 AM, Thom Brown <thom@linux.com> wrote: > On 29 March 2012 16:30, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote: >> Robert Haas <robertmhaas@gmail.com> writes: >>> On Thu, Mar 29, 2012 at 9:01 AM, Thom Brown <thom@linux.com> wrote: >>>> On 29 March 2012 13:30, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote: >>>>> I'll go make that happen, and still need input here. We first want to >>>>> have command triggers on specific commands or ANY command, and we want >>>>> to implement 3 places from where to fire them. >>>>> >>>>> Here's a new syntax proposal to cope with that: >>>> >>>> Is it necessary to add this complexity in this version? Can't we keep >>>> it simple but in a way that allows the addition of this later? The >>>> testing of all these new combinations sounds like a lot of work. >>> >>> I concur. This is way more complicated than we should be trying to do >>> in version 1. >> >> I'm at a loss here. This proposal was so that we can reach a commonly >> agreed minimal solution and design in first version. There's no new >> piece of infrastructure to add, the syntax is changed only to open the >> road for later, I'm not changing where the current command triggers are >> to be called (except for those which are misplaced). >> >> So, please help me here: what do we want to have in 9.3? > > Perhaps I misunderstood. It was the addition of the fine-grained even > options (parse, execute etc) I saw as new. If you're saying those are > just possible options for later that won't be in this version, I'm > fine with that. If those are to make it for 9.2, then creating the > necessary test cases and possible fixes sounds infeasible in such a > short space of time. Please disregard if this is not the case. So... I've said repeatedly and over a long period of time that development of this feature wasn't started early enough in the cycle to get it finished in time for 9.2. I think that I've identified some pretty serious issues in the discussion we've had so far, especially (1) the points at which figures are fired aren't consistent between commands, (2) not much thought has been given to what happens if, say, a DDL trigger performs a DDL operation on the table the outer DDL command is due to modify, and (3) we are eventually going to want to trap a much richer set of events than can be captured by the words "before" and "after". Now, you could view this as me throwing up roadblocks to validate my previously-expressed opinion that this wasn't going to get done, but I really, honestly believe that these are important issues and that getting them right is more important than getting something done now. If we still want to try to squeeze something into 9.2, I recommend stripping out everything except for what Dimitri called the before-any-command firing point. In other words, add a way to run a procedure after parsing of a command but before any name lookups have been done, any permissions checks have been done, or any locks have been taken. The usefulness of such a hook is of course limited but it is also a lot less invasive than the patch I recently reviewed and probably a lot safer. I actually think it's wise to do that as a first step even if it doesn't make 9.2, because it is much easier to build features like this incrementally and even a patch that does that will be reasonably complicated and difficult to review. Parenthetically, what Dimitri previously called the after-any-command firing point, all the way at the end of the statement but without any specific details about the object the statement operated on, seems just as good for a first step, maybe better, so that would be a fine foundation from my point of view as well. The stuff that happens somewhere in the middle, even just after locking and permissions checking, is more complex and I think that should be phase 2 regardless of which phase ends up in which release cycle. I'm not sure whether Dimitri's question about 9.3 was a typo for 9.2 or whether that's what he was actually asking about. But to answer that question for 9.3, I think we have time to build an extremely extensive infrastructure that covers a huge variety of use cases, and I think there is hardly any reasonable proposal that is off the table as far as that goes. We have a year to get that work done, and a year is a long time, and with incremental progress at each CommitFest we can do a huge amount. I can also say that I would be willing to put in some serious work during the 9.3 cycle to accelerate that progress, too, especally if (ahem) some other people can return the favor by reviewing my patches. :-) I could see us adding the functionality described above in one CommitFest and then spending the next three adding more whiz-bango frammishes and ending up with something really nice. Right now, though, we are very crunched for time, and probably shouldn't be entertaining anything that requires a tenth the code churn that this patch probably does; if we are going to do anything at all, it had better be as simple and uninvasive as we can make it. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
pgsql-hackers by date: