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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
