Vik Fearing <vik@postgresfriends.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
