Re: Review: pre-commit triggers

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Ian Lawrence Barwick <barwick(at)gmail(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Andrew Dunstan <andrew(at)dunslane(dot)net>
Subject: Re: Review: pre-commit triggers
Date: 2013-11-19 15:58:22
Message-ID: CA+TgmoY9Uv7SfHbqyR7m=ma+5ZYLEhUNAO9hxDiELqRkMeVr9g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Nov 18, 2013 at 9:39 AM, Ian Lawrence Barwick <barwick(at)gmail(dot)com> wrote:
> 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.

Ending up in a failed transaction block would be wrong. If the user
does a BEGIN, a bunch of stuff, and a COMMIT, they're entitled to
assume without checking that they are no longer in a transaction
block. The COMMIT may have actually performed a ROLLBACK, but one way
or the other the transaction block will have ended. This is important
for things like psql <
my-dumb-script-with-several-begin-commit-blocks.

It is a little less clear whether it's best for the COMMIT to return
an ERROR message or something else, but I think the ERROR is probably
the best solution. There is already commit-time code that can fail
today, so there should be precedent here. And I suspect anything
other than ERROR will be really messy to implement.

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2013-11-19 15:59:02 Re: nested hstore patch
Previous Message Bruce Momjian 2013-11-19 15:55:59 Re: nested hstore patch