Re: Review: pre-commit triggers

On 11/18/2013 09:39 AM, Ian Lawrence Barwick wrote:
> Review for Andrew Dunstan's patch in CF 2013-11:
>
>    https://commitfest.postgresql.org/action/patch_view?id=1312
>
> This review is more from a usage point of view, I would like
> to spend more time looking at the code but only so many hours in a day,
> etcetera; I hope this is useful as-is.


Many thanks for the fast review.

>
> * Does it include reasonable tests, necessary doc patches, etc?
> Documentation patches included, but no tests. (I have a feeling it
> might be necessary to add a FAQ somewhere as to why there's
> no transaction rollback trigger).

I'll add some tests very shortly, and see about adding that to the docs.


> * Are there corner cases the author has failed to consider?
>
> I'm not sure whether it's intended behaviour, but if the
> commit trigger itself fails, there's an implicit rollback, e.g.:
>
>    postgres=# BEGIN ;
>    BEGIN
>    postgres=*# INSERT INTO foo (id) VALUES (1);
>    INSERT 0 1
>    postgres=*# COMMIT ;
>    NOTICE:  Pre-commit trigger called
>    ERROR:  relation "bar" does not exist
>    LINE 1: SELECT foo FROM bar
>   ^
>    QUERY:  SELECT foo FROM bar
>    CONTEXT:  PL/pgSQL function pre_commit_trigger() line 4 at EXECUTE statement
>    postgres=#
>
> I'd expect this to lead to a failed transaction block,
> or at least some sort of notice that the transaction itself
> has been rolled back.


I'll check on this.

>
> Note that 'DROP FUNCTION broken_trigger_function() CASCADE' will remove
> the broken function without having to resort to single user mode so
> it doesn't seem like an error in the commit trigger itself will
> necessarily lead to an intractable situation.


Given that, do we want to keep the bar on these operating in single user 
mode? I can easily drop it and just document this way out of difficulty.


cheers

andrew