Re: Event Triggers reduced, v1 - Mailing list pgsql-hackers
From | Robert Haas |
---|---|
Subject | Re: Event Triggers reduced, v1 |
Date | |
Msg-id | CA+TgmoYT30PZwibaXP1V8=ogPKX4T69qE9zR5kwyb73Th5bD2g@mail.gmail.com Whole thread Raw |
In response to | Re: Event Triggers reduced, v1 (Dimitri Fontaine <dimitri@2ndQuadrant.fr>) |
Responses |
Re: Event Triggers reduced, v1
(Dimitri Fontaine <dimitri@2ndQuadrant.fr>)
Re: Event Triggers reduced, v1 (Robert Haas <robertmhaas@gmail.com>) |
List | pgsql-hackers |
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. I made the event triggers stuff its own chapter rather than trying to fold it in under triggers, and added some more detail. It's already quite a bit of extra stuff, and it's only going to become more as we expand this feature, so I think a separate chapter is appropriate. I moved a bunch of the details that were under CREATE EVENT TRIGGER into this new chapter, which I think is a better location, and reformatted the matrix somewhat. I think as we add more firing points it will be clearer and easier to read if we have all the commands arranged in columns rather than listing a bunch of firing points on each line. I also made a bunch of minor edits to improve readability and improve the English (which wasn't bad, but I touched it up a bit); and I tried to add some extra detail here and there (some of it recycled from previous patch versions). Assuming this all seems reasonably agreeable, can you merge it on your side? This took the last several hours, so I haven't looked at your latest code changes yet. However, in the course of editing the documentation, it occurred to me that we seem to be fairly arbitrarily excluding a large number of commands from the event trigger mechanism. For example, GRANT and REVOKE. In earlier patches, we needed specific changes for every command, so there was some reason not to try to support everything right out of the gate. But ISTM that the argument for this is much less now; presumably it's just a few extra lines of code per command, so maybe we ought to go ahead and try to make this as complete as possible. I attempt to explain in the attached patch the reasons why we don't support certain classes of commands, but I can't come up with any explanation for supporting GRANT and REVOKE that doesn't fall flat. I can't even really see a reason not to support things like LISTEN and NOTIFY, and it would certainly be more consistent with the notion of a command_start trigger to support as many commands as we can. I had an interesting experience while testing this patch. I accidentally redefined my event trigger function to something which errored out. That of course precluded me from using CREATE OR REPLACE FUNCTION to fix it. This makes me feel rather glad that we decided to exclude CREATE/ALTER/DROP EVENT TRIGGER from the event trigger mechanism, else recovery would have had to involve system catalog hackery. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Attachment
pgsql-hackers by date: