Re: Event Triggers reduced, v1 - Mailing list pgsql-hackers

From Robert Haas
Subject Re: Event Triggers reduced, v1
Date
Msg-id CA+TgmobiS2faeKOFGP1CkT2wY=ZMafRGCbE5jp7HQiMnG_Z=dQ@mail.gmail.com
Whole thread Raw
In response to Re: Event Triggers reduced, v1  (Robert Haas <robertmhaas@gmail.com>)
Responses Re: Event Triggers reduced, v1  (Dimitri Fontaine <dimitri@2ndQuadrant.fr>)
List pgsql-hackers
On Fri, Jul 6, 2012 at 12:00 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Fri, Jul 6, 2012 at 7:21 AM, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote:
>> Robert Haas <robertmhaas@gmail.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

Attachment

pgsql-hackers by date:

Previous
From: Dimitri Fontaine
Date:
Subject: Re: Event Triggers reduced, v1
Next
From: Dimitri Fontaine
Date:
Subject: Re: Event Triggers reduced, v1