Re: Event Triggers: adding information

On Tue, Jan 22, 2013 at 7:29 AM, Dimitri Fontaine
<dimitri(at)2ndquadrant(dot)fr> wrote:
> Dimitri Fontaine <dimitri(at)2ndQuadrant(dot)fr> writes:
>> I added some in the attached patch.
>>
>> doc/src/sgml/event-trigger.sgml | 10 ++
>> src/backend/commands/event_trigger.c | 6 +-
>> src/test/regress/expected/event_trigger.out | 106 +++++++++++++++++++
>> src/test/regress/sql/event_trigger.sql | 47 ++++++++
>> 4 files changed, 167 insertions(+), 2 deletions(-)
>
> And I did drop a comment line I didn't mean to when trying things out,
> so here's the update copy. There's a bug fix in there too, in both the
> versions of the patch, that the new regression tests exercise.

I think these new regression tests are no good, because I doubt that
the number of recursive calls that can fit into any given amount of
stack space is guaranteed to be the same on all platforms. I have
committed the bug fixes themselves, however.

I wasn't entirely happy with your proposed documentation so I'm
attaching a counter-proposal. My specific complaints are (1) telling
people that event triggers are run in a savepoint seems a little too
abstract; I have attempted to make the consequences more concrete; (2)
RAISE EXCEPTION is PL/pgsql specific and not the only possible reason
for an error; I have attempted to be more generic; and (3) in the
process of fiddling with this, I noticed that the ddl_command_end
documentation can, I believe, be made both shorter and more clear by
turning it into a rider on the previous paragraph.

Comments?

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