regression in transaction behavior in 1.29.0, savepoints now impossible

[zzzeek]

hey there -

I tried to see if there were maybe ways to work around this in our test suite but actually there's not, the change at b2c5d6f7079ae9004e316c9e6536fa39cfb94f54 prevents any SQL whatsoever from being emitted including ROLLBACK TO SAVEPOINT, which is the usual way we deal with being able to have database errors handled without rolling back the entire transaction.

PostgreSQL already ignores invalid commands in transaction blocks so if it were me, I'd fully revert this behavior; no other DBAPI does this. I see a rationale at #36 but this is not something any other PG driver does, psycopg2, psycopg3, asyncpg, etc. It's not a problem to be solved by the driver.

in the example below, we wish to use SAVEPOINT to emit SQL that might fail. with the new change, we are unable to ROLLBACK TO SAVEPOINT when the failure occurs.

import pg8000 as dbapi

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

cursor = conn.cursor()

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

cursor.execute("ROLLBACK TO SAVEPOINT sa_1")

output:

Traceback (most recent call last):
  File "/home/classic/dev/sqlalchemy/test3.py", line 15, in <module>
    cursor.execute("ROLLBACK TO SAVEPOINT sa_1")
  File "/home/classic/.venv3/lib/python3.10/site-packages/pg8000/legacy.py", line 251, in execute
    self._context = self._c.execute_simple(operation)
  File "/home/classic/.venv3/lib/python3.10/site-packages/pg8000/core.py", line 659, in execute_simple
    self.handle_messages(context)
  File "/home/classic/.venv3/lib/python3.10/site-packages/pg8000/core.py", line 813, in handle_messages
    raise context.error
pg8000.exceptions.InterfaceError: in failed transaction block

[tlocke]

Hi @zzzeek, thanks for spotting this. I've quickly done release 1.29.1 that should allow ROLLBACK TO SAVEPOINT. I'll have a think about what to do in the longer term when I've got a bit more time.

[zzzeek]

OK, thanks for that. I can now see the pg8000 error providing cover for PostgreSQL transactions that are aborted in any case, so I guess it's a good thing even though i have to fix more things in our test suite.