cs50/python-cs50

SQL execute does not autocomplete transaction if an IntegrityError occurs

Closed this issue · 4 comments

In SQL.execute, if autocommit is on (default behavior) and the SQL query fails due to an IntegrityError, the open transaction is never closed via COMMIT or ROLLBACK. Relevant snippet:

329                # Execute statement
330                if self._autocommit:
331                    connection.execute(sqlalchemy.text("BEGIN"))
332                result = connection.execute(sqlalchemy.text(statement))
333                if self._autocommit:
334                    connection.execute(sqlalchemy.text("COMMIT"))

In this case, on line 331, a transaction is started, on 332 the statement fails, and thus line 334 is never executed. In the except clause, the transaction never gets completed either.

Then, when the next query is run, the BEGIN on line 331 causes RuntimeError: cannot start a transaction within a transaction because the previous transaction was never closed.

Being able to catch an IntegrityError is important for preventing race conditions (if the query fails, find out why as opposed to check if the record already exists, and then insert)

Minimal reproducible example (tested with cs50-9.3.0 on Ubuntu 20.04 (Python 3.8) and Windows 10 (Python 3.11):

from cs50 import SQL

open("test.db", "w").close()

db = SQL("sqlite:///test.db")
db.execute("CREATE TABLE test(id integer UNIQUE)")
db.execute("INSERT INTO test VALUES(1)")
try:
  # this should fail because of the UNIQUE constraint
  db.execute("INSERT INTO test VALUES(1)")
except ValueError as e:
  print(e)
print(db.execute("SELECT * FROM test"))

Expected output:

UNIQUE constraint failed: test.id
[{'id': 1}]

Actual output:

UNIQUE constraint failed: test.id
Traceback (most recent call last):
  File "\tmp\venv\test.py", line 13, in <module>
    print(db.execute("SELECT * FROM test"))
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "\tmp\venv\lib\site-packages\cs50\sql.py", line 29, in decorator
    return f(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^
  File "\tmp\venv\lib\site-packages\cs50\sql.py", line 413, in execute
    raise e
RuntimeError: cannot start a transaction within a transaction

Thanks for the detailed report!

Upon further testing, it seems like the upgrade to SQLAlchemy causes this bug: in 1.4.49, the transaction gets aborted when the IntegrityError is raised, and in 2.0.23, the transaction is not aborted. The sample code I provided initially does produce the right output with SQLAlchemy 1.4.49, but not with 2.0.23.

Thanks for the fix! I think you should update setup.py to require SQLAlchemy==2 since your fix breaks the sample code when using SQLAlchemy v1

Ah, thanks!