Re: Error on failed COMMIT

On Tue, 11 Feb 2020 at 17:35, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Vik Fearing <vik(at)postgresfriends(dot)org> writes:
> > There is a current discussion off-list about what should happen when a
> > COMMIT is issued for a transaction that cannot be committed for whatever
> > reason. PostgreSQL returns ROLLBACK as command tag but otherwise
> succeeds.
>
> > It seems like [ trying to commit a failed transaction ]
> > should actually produce something like this:
>
> > postgres=!# commit;
> > ERROR: 40P00: transaction cannot be committed
> > DETAIL: First error was "42601: syntax error at or near "error""
>
> So I assume you're imagining that that would leave us still in
> transaction-aborted state, and the session is basically dead in
> the water until the user thinks to issue ROLLBACK instead?
>
> > Is this reading correct?
>
> Probably it is, according to the letter of the SQL spec, but I'm
> afraid that changing this behavior now would provoke lots of hate
> and few compliments. An application that's doing the spec-compliant
> thing and issuing ROLLBACK isn't going to be affected, but apps that
> are relying on the existing behavior are going to be badly broken.
>
> A related problem is what happens if you're in a perfectly-fine
> transaction and the commit itself fails, e.g.,
>
> regression=# create table tt (f1 int primary key deferrable initially
> deferred);
> CREATE TABLE
> regression=# begin;
> BEGIN
> regression=# insert into tt values (1);
> INSERT 0 1
> regression=# insert into tt values (1);
> INSERT 0 1
> regression=# commit;
> ERROR: duplicate key value violates unique constraint "tt_pkey"
> DETAIL: Key (f1)=(1) already exists.
>
> At this point PG considers that you're out of the transaction:
>
> regression=# rollback;
> WARNING: there is no transaction in progress
> ROLLBACK
>

interesting as if you do a commit after violating a not null it simply does
a rollback
with no warning whatsoever

begin;
BEGIN
test=# insert into hasnull(i) values (null);
ERROR: null value in column "i" violates not-null constraint
DETAIL: Failing row contains (null).
test=# commit;
ROLLBACK

>
> but I bet the spec doesn't. So if we change that, again we break
> applications that work today. Meanwhile, an app that is doing it
> the spec-compliant way will issue a ROLLBACK that we consider
> useless, so currently that draws an ignorable WARNING and all is
> well. So here also, the prospects for making more people happy
> than we make unhappy seem pretty grim. (Maybe there's a case
> for downgrading the WARNING to NOTICE, though?)
>
> Actually the bug reporter was looking for an upgrade from a warning to an
ERROR

I realize we are unlikely to change the behaviour however it would be
useful if we
did the same thing for all cases, and document this behaviour. We actually
have places where
we document where we don't adhere to the spec.

Dave