Re: Command Triggers patch v18

On Thu, Mar 29, 2012 at 11:49 AM, Thom Brown <thom(at)linux(dot)com> wrote:
> On 29 March 2012 16:30, Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr> wrote:
>> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>>> On Thu, Mar 29, 2012 at 9:01 AM, Thom Brown <thom(at)linux(dot)com> wrote:
>>>> On 29 March 2012 13:30, Dimitri Fontaine <dimitri(at)2ndquadrant(dot)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