error on failed commit

[davecramer]

@tlocke What are you doing about the problem that PostgreSQL will silently rollback a failed commit. I am one of the pgjdbc authors and we are planning on checking for rollback and issuing an error when this happens see https://www.postgresql.org/message-id/CADK3HHJmF7N74jCasC+Ym6ZTDxq5LrB_9MHb+=pHoX6XzoHbLQ@mail.gmail.com for a discussion

[tlocke]

Hi @davecramer, I wasn't aware of this problem, thanks for pointing it out to me. I've had a read through the thread and my thinking is that a commit should always error if it fails, so I agree that the server should be changed to do this. I understand why people are wary of making a change to the server's behaviour, but in this case I'm convinced it'll do more good than harm, because it brings it in line with how people thought it was behaving anyway.

[davecramer]

Feel free to jump on the thread and let them know

…

On Thu, Apr 16, 2020, 3:00 PM Tony Locke, ***@***.***> wrote: Hi @davecramer <https://github.com/davecramer>, I wasn't aware of this problem, thanks for pointing it out to me. I've had a read through the thread and my thinking is that a commit should *always* error if it fails, so I agree that the server should be changed to do this. I understand why people are wary of making a change to the server's behaviour, but in this case I'm convinced it'll do more good than harm, because it brings it in line with how people thought it was behaving anyway. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#36 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADDH5UILLEXVDJOWGYEOTLRM5ITRANCNFSM4MJ2TFHA> .

[tlocke]

I've messaged the mailing list with my view, supporting the patch.

[davecramer]

If you know any other driver/client writers please pass this along. The server folks seem to be a bit myopic.

[tlocke]

AFAIK there was a patch to this for the server, but it turned out to have some unforeseen problems? Anyway, I thought I'd close this issue, but feel free to re-open if needed.

[tlocke]

To summarize: there is a longstanding bug in the PostgreSQL server whereby if a COMMIT is issued against a failed transaction, the transaction is silently rolled back, rather than an error being returned. pg8000 attempts to detect when this has happened and raise an InterfaceError. Since it's a bug in the server, the problem affects all clients. Here's an example in psql:

tlocke=> create temporary table tt (f1 int primary key);
CREATE TABLE
tlocke=> begin;
BEGIN
tlocke=> insert into tt(f1) values(null);
ERROR:  null value in column "f1" violates not-null constraint
DETAIL:  Failing row contains (null).
tlocke=> commit;
ROLLBACK
tlocke=> 

Note that on the commit; there is no error, the failed transaction is silently rolled back.

The logic that pg8000 follows to work out if there's been a silent failed commit, is held in the CommandComplete handler and it raises an InterfaceError if the following hold:

• The transaction status from the previous ReadyForQuery is E.
• No ErrorResponse has been received since the last command.
• The query text is not ROLLBACK (allowing for possible whitespace either side and a final ;)

In my view, the above is a workaround for a bug in the PostgreSQL server, and the server should really return an ErrorResponse if a commit is issued against a failed transaction. If anyone can help get it fixed in the server that would be much appreciated!

[zzzeek]

can we assume b2c5d6f was committed in the interests of this issue? there's no cross-linking.

pg8000 has released 1.29.0 so it's in your court to introduce this new behavior, I would note however that no DBAPI I've ever worked with has this behavior, including all the other PG drivers:

import pg8000 as dbapi
# import psycopg2


conn = dbapi.connect(user="scott", password="tiger", host="localhost", database="test")

cursor = conn.cursor()

try:
    cursor.execute("delete from nonexistent")
except:
    pass

# fails with error 
conn.commit()

[zzzeek]

actually this prevents savepoints from working. I'll open a new issue.

[davecramer]

I agree that this is a bug in the server. I attempted to provide a patch once but it was not well received. See https://www.postgresql.org/message-id/b9fb50dc-0f6e-15fb-6555-8ddb86f4aa71%40postgresfriends.org for discussion