Re: Event Triggers reduced, v1

On Fri, Jul 6, 2012 at 12:00 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Fri, Jul 6, 2012 at 7:21 AM, Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr> wrote:
>> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>>> Attached is a incremental patch with a bunch of minor cleanups,
>>> including reverts of a few spurious white space changes. Could you
>>> merge this into your version?
>>
>> Thank you very much for that, yes it's included now. So you have 3
>> attachments here, the whole new patch revision (v1.7), the incremental
>> patch to go from 1.6 to 1.7 and the incremental patch that should apply
>> cleanly on top of your cleanups.
>
> Here is an incremental documentation patch which I hope you will like.

And here is another incremental patch, this one doing some more
cleanup. Some of this is cosmetic, but it also:

- Fixes the new event_trigger type so that it passes the type sanity
test, instead of adding the failure as expected output.
- Fixes DROP EVENT TRIGGER IF EXISTS on a non-existent trigger.
- Fleshes out the ownership handling so that it's more similar to what
we do for other types of objects.

I'm feeling pretty good about this at this point, although I think
there is still some more work to do before we call it done and go
home.

I have a large remaining maintainability concern about the way we're
mapping back and forth between node tags, event tags, and command
tags. Right now we've got parse_event_tag, which parses something
like 'ALTER AGGREGATE' into E_AlterAggregate; and then we've got
command_to_string, which turns E_AlterAggregate back into 'ALTER
AGGREGATE', and then we've got InitEventContext(), which turns
T_RenameStmt or T_AlterObjectSchemaStmt with OBJECT_AGGREGATE into
E_AlterAggregate. I can't easily verify that all three of these
things are consistent with each other, and even if they are right now
I estimate the chances of that remaining true as other people patch
the code as near-zero. You didn't like my last proposal for dealing
with this, which is fine: it might not have been the best way of
dealing with it. But I think we have to figure out something better
than what we've got now, or this is almost guaranteed to get broken.
If you don't have a brilliant idea I'll hack on it and see what I can
come up with.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company