From adfcbb4731da5ba425a760c9a48be6d77ad7856e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 9 Jul 2026 22:06:46 +0000 Subject: [PATCH] Support sqlite3.connect(autocommit=False) connections too Database() now accepts all three Python transaction handling modes and behaves identically in each. For autocommit=False connections - where the driver holds an implicit transaction open at all times - the library tracks transaction ownership itself: - A new _explicit_transaction flag distinguishes transactions opened with begin()/atomic() from the driver's implicit one, exposed as a new db.in_transaction property (conn.in_transaction is always True in this mode) - begin() claims the driver's implicit transaction instead of executing BEGIN, which the driver would reject; BEGIN/COMMIT/ ROLLBACK passed to db.execute() are routed through begin()/commit()/ rollback() - Writes outside a user transaction commit the implicit transaction immediately, preserving the library's auto-commit contract - Row-returning statements outside a transaction fetch eagerly and commit, so the implicit read transaction does not hold a shared lock that blocks writes from other connections - PRAGMA and VACUUM run in temporary driver autocommit mode (ensure_autocommit_on() now flips conn.autocommit), since PRAGMAs are silently ignored and VACUUM refused inside the implicit transaction A new pytest --sqlite-autocommit-false option runs the entire suite in this mode, wired into CI alongside --sqlite-autocommit. Tests that asserted on conn.in_transaction or wrote through db.conn without committing now use the mode-aware library API instead. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PFkrS2nuP8mo1jmT594VCd --- .github/workflows/test.yml | 4 +- docs/changelog.rst | 2 +- docs/python-api.rst | 16 ++- docs/upgrading.rst | 2 +- sqlite_utils/db.py | 246 +++++++++++++++++++++++++++++++---- tests/conftest.py | 23 +++- tests/test_analyze_tables.py | 5 + tests/test_atomic.py | 34 ++--- tests/test_cli.py | 4 +- tests/test_constructor.py | 30 ++--- tests/test_delete.py | 2 +- tests/test_fts.py | 2 +- tests/test_query.py | 24 ++-- tests/test_transform.py | 10 +- tests/test_wal.py | 13 +- 15 files changed, 323 insertions(+), 94 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 923de2e..2393484 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -40,7 +40,9 @@ jobs: pytest -v - name: Run autocommit tests just on 3.14/Ubuntu if: matrix.os == 'ubuntu-latest' && matrix.python-version == '3.14' - run: pytest --sqlite-autocommit + run: | + pytest --sqlite-autocommit + pytest --sqlite-autocommit-false - name: run mypy run: mypy sqlite_utils tests - name: run flake8 diff --git a/docs/changelog.rst b/docs/changelog.rst index d2ba16a..1dae627 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -13,7 +13,7 @@ Unreleased - ``sqlite-utils insert`` and ``sqlite-utils upsert`` now accept a ``--code`` option for :ref:`providing a block of Python code ` (or a path to a ``.py`` file) that defines a ``rows()`` function or ``rows`` iterable of rows to insert, as an alternative to importing from a file. (:issue:`684`) - ``sqlite-utils insert`` and ``sqlite-utils upsert`` now accept ``--type column-name type`` to :ref:`override the type automatically chosen when the table is created `. This is useful for CSV or TSV columns such as ZIP codes that look like integers but should be stored as ``TEXT`` to preserve leading zeros. (:issue:`131`) - New ``table.drop_index(name)`` method and ``sqlite-utils drop-index`` command for dropping an index by name. Both accept ``ignore=True``/``--ignore`` to ignore a missing index. (:issue:`626`) -- ``Database()`` now accepts connections created with the Python 3.12+ ``sqlite3.connect(..., autocommit=True)`` option. The library manages transactions using explicit ``BEGIN``/``COMMIT``/``ROLLBACK`` and savepoint statements, which behave identically in that mode. Connections created with ``autocommit=False`` are still rejected with a ``TransactionError``, because the driver holds an implicit transaction open at all times in that mode, breaking explicit transaction handling. See :ref:`python_api_transactions_modes`. +- ``Database()`` now accepts connections created with the Python 3.12+ ``sqlite3.connect(..., autocommit=True)`` and ``autocommit=False`` options, and adapts its transaction handling so every connection mode behaves identically. Writes still commit automatically unless a transaction opened with ``db.begin()`` or ``db.atomic()`` is open. On ``autocommit=False`` connections - where the driver holds an implicit transaction open at all times - row-returning statements executed outside of a transaction are fetched eagerly so their read transactions do not hold locks that block other connections, and ``PRAGMA``/``VACUUM`` statements run in temporary driver-level autocommit mode. A new ``db.in_transaction`` property reports whether a ``begin()``/``atomic()`` transaction is open, which ``conn.in_transaction`` cannot answer for ``autocommit=False`` connections. See :ref:`python_api_transactions_modes`. .. _v4_0: diff --git a/docs/python-api.rst b/docs/python-api.rst index 1e72c1c..9b3d69e 100644 --- a/docs/python-api.rst +++ b/docs/python-api.rst @@ -95,7 +95,7 @@ Instead of a file path you can pass in an existing SQLite connection: db = Database(sqlite3.connect("my_database.db")) -The connection can use Python's default transaction handling or the Python 3.12+ ``sqlite3.connect(..., autocommit=True)`` mode. Connections created with ``autocommit=False`` are rejected with a ``sqlite_utils.db.TransactionError`` - see :ref:`python_api_transactions_modes`. +The connection can use Python's default transaction handling, or the Python 3.12+ ``sqlite3.connect(..., autocommit=True)`` or ``autocommit=False`` modes - see :ref:`python_api_transactions_modes`. If you want to create an in-memory database, you can do so like this: @@ -409,9 +409,19 @@ Two related safeguards to be aware of: Supported connection modes -------------------------- -``db.atomic()`` and the automatic per-method transactions work with connections in Python's default transaction handling mode and with connections created using the Python 3.12+ ``sqlite3.connect(..., autocommit=True)`` option. The library manages transactions itself using explicit ``BEGIN``, ``COMMIT``, ``ROLLBACK`` and savepoint statements, which behave identically in both modes. +``db.atomic()`` and the automatic per-method transactions work with connections in all three of Python's transaction handling modes: the default mode, and the Python 3.12+ ``sqlite3.connect(..., autocommit=True)`` and ``autocommit=False`` modes. -Passing a connection created with ``autocommit=False`` to ``Database()`` raises a ``sqlite_utils.db.TransactionError``. In that mode the ``sqlite3`` driver holds an implicit transaction open at all times, which breaks the explicit transaction handling used by every write method - for example ``BEGIN`` fails because a transaction is always already open. +In the default mode and with ``autocommit=True`` the library manages transactions itself using explicit ``BEGIN``, ``COMMIT``, ``ROLLBACK`` and savepoint statements, which behave identically in both modes. + +With ``autocommit=False`` the ``sqlite3`` driver holds an implicit transaction open at all times, so the library adapts to preserve the same behavior: + +- Writes made through the library still commit automatically, unless a transaction opened with ``db.begin()`` or ``db.atomic()`` is open. ``db.begin()`` claims the driver's implicit transaction rather than executing ``BEGIN``, which the driver would reject. +- ``BEGIN``, ``COMMIT`` and ``ROLLBACK`` statements passed to ``db.execute()`` are routed through ``db.begin()``, ``db.commit()`` and ``db.rollback()``. +- ``PRAGMA`` and ``VACUUM`` statements executed outside of a transaction run in temporary driver-level autocommit mode, because both are silently ignored - or refused - inside the driver's implicit transaction. +- Row-returning statements executed with ``db.execute()`` or ``db.query()`` outside of a transaction fetch their results eagerly, then commit. A lazily-read cursor would hold the implicit read transaction - and its shared database lock, which blocks writes from other connections - open indefinitely. This means very large result sets are buffered in memory in this mode. +- Statements executed directly on ``db.conn`` are not managed: the driver's own rules apply, and nothing is committed until you call ``db.conn.commit()`` yourself. + +Use the ``db.in_transaction`` property to check whether a transaction opened with ``db.begin()`` or ``db.atomic()`` is currently open - on ``autocommit=False`` connections ``db.conn.in_transaction`` is always ``True``, so it cannot answer that question. .. _python_api_table: diff --git a/docs/upgrading.rst b/docs/upgrading.rst index da9a3e9..697290e 100644 --- a/docs/upgrading.rst +++ b/docs/upgrading.rst @@ -109,7 +109,7 @@ Two related behavior changes to ``table.transform()``: compound foreign keys now - Multi-step operations such as ``table.transform()`` no longer commit an existing transaction you have open - they use savepoints inside it instead. - ``db.enable_wal()`` and ``db.disable_wal()`` raise a ``sqlite_utils.db.TransactionError`` if called while a transaction is open, instead of silently committing it. - Using ``Database`` as a context manager (``with Database(path) as db:``) closes the connection on exit *without* committing - a transaction you explicitly opened with ``db.begin()`` and did not commit is rolled back. -- ``Database()`` rejects connections created with the Python 3.12+ ``sqlite3.connect(..., autocommit=False)`` option, raising ``sqlite_utils.db.TransactionError``. In that mode the driver holds an implicit transaction open at all times, breaking the library's explicit transaction handling. Connections created with ``autocommit=True`` are supported. +- ``Database()`` accepts connections created with the Python 3.12+ ``sqlite3.connect(..., autocommit=True)`` and ``autocommit=False`` options, and adapts its transaction handling so all connection modes behave identically - see :ref:`python_api_transactions_modes`. Packaging changes ----------------- diff --git a/sqlite_utils/db.py b/sqlite_utils/db.py index 25029cc..37f9f38 100644 --- a/sqlite_utils/db.py +++ b/sqlite_utils/db.py @@ -479,6 +479,48 @@ def _first_keyword(sql: str) -> str: return sql[i:j].upper() +class _PrefetchedCursor: + """ + Stands in for a ``sqlite3.Cursor`` whose rows were fetched eagerly, so + that the driver's implicit read transaction could be committed - used + for row-returning statements on Python 3.12+ ``autocommit=False`` + connections, where a lazily-read cursor would hold the read transaction + (and its shared lock) open indefinitely. + """ + + def __init__(self, cursor: sqlite3.Cursor, rows: list): + self.description = cursor.description + self.lastrowid = cursor.lastrowid + self.rowcount = cursor.rowcount + self.arraysize = cursor.arraysize + self._rows = iter(rows) + + def __iter__(self) -> "_PrefetchedCursor": + return self + + def __next__(self): + return next(self._rows) + + def fetchone(self): + return next(self._rows, None) + + def fetchmany(self, size: Optional[int] = None) -> list: + size = size or self.arraysize + results = [] + for _ in range(size): + row = next(self._rows, None) + if row is None: + break + results.append(row) + return results + + def fetchall(self) -> list: + return list(self._rows) + + def close(self) -> None: + pass + + class Database: """ Wrapper for a SQLite database connection that adds a variety of useful utility methods. @@ -526,6 +568,10 @@ class Database: self.memory_name = None self.memory = False self.use_old_upsert = use_old_upsert + # Tracks transaction ownership for autocommit=False connections, + # where conn.in_transaction cannot distinguish the driver's implicit + # transaction from one opened with begin() or atomic() + self._explicit_transaction = False if not ( (filename_or_conn is not None and (not memory and not memory_name)) or (filename_or_conn is None and (memory or memory_name)) @@ -557,17 +603,6 @@ class Database: if recreate: raise ValueError("recreate cannot be used with connections, only paths") self.conn = cast(sqlite3.Connection, filename_or_conn) - # Python 3.12+ autocommit=False connections hold an implicit - # transaction open at all times, which breaks the explicit - # BEGIN/COMMIT transaction handling used by every write method. - # autocommit=True connections work fine - the library manages - # transactions itself with explicit SQL statements - if getattr(self.conn, "autocommit", None) is False: - raise TransactionError( - "sqlite-utils does not support connections created with " - "autocommit=False - use autocommit=True or the default " - "transaction handling instead" - ) self._tracer: Optional[Tracer] = tracer if recursive_triggers: self.execute("PRAGMA recursive_triggers=on;") @@ -593,6 +628,34 @@ class Database: "Close the SQLite connection, and the underlying database file" self.conn.close() + @property + def _autocommit_false(self) -> bool: + # True for Python 3.12+ sqlite3.connect(autocommit=False) connections, + # where the driver holds an implicit transaction open at all times + return getattr(self.conn, "autocommit", None) is False + + def _commit_if_in_transaction(self) -> None: + # On autocommit=False connections conn.commit() raises if no + # transaction is active - which can happen after an error such as a + # RAISE(ROLLBACK) trigger destroys the transaction + if self.conn.in_transaction: + self.conn.commit() + + @property + def in_transaction(self) -> bool: + """ + ``True`` if a transaction opened with :meth:`begin` or :meth:`atomic` + is currently open. + + Unlike ``conn.in_transaction`` this is meaningful for every connection + mode - on Python 3.12+ ``autocommit=False`` connections the driver + keeps an implicit transaction open at all times, so + ``conn.in_transaction`` is always ``True`` there. + """ + if self._autocommit_false: + return self._explicit_transaction + return self.conn.in_transaction + @contextlib.contextmanager def atomic(self) -> Generator["Database", None, None]: """ @@ -600,7 +663,7 @@ class Database: Nested blocks use SQLite savepoints. """ - if self.conn.in_transaction: + if self.in_transaction: savepoint = "sqlite_utils_{}".format(secrets.token_hex(16)) self.conn.execute("SAVEPOINT {};".format(savepoint)) try: @@ -617,7 +680,12 @@ class Database: else: self.conn.execute("RELEASE SAVEPOINT {};".format(savepoint)) else: - self.conn.execute("BEGIN") + if self._autocommit_false: + # The driver already holds an open implicit transaction - + # claim it, so writes stop committing automatically + self._explicit_transaction = True + else: + self.conn.execute("BEGIN") try: yield self except BaseException: @@ -627,7 +695,11 @@ class Database: raise else: try: - self.conn.execute("COMMIT") + if self._autocommit_false: + self._explicit_transaction = False + self._commit_if_in_transaction() + else: + self.conn.execute("COMMIT") except BaseException: self.rollback() raise @@ -642,12 +714,25 @@ class Database: Most code should use the :meth:`atomic` context manager instead, which commits and rolls back automatically. See :ref:`python_api_transactions`. """ + if self._autocommit_false: + if self._explicit_transaction: + raise sqlite3.OperationalError( + "cannot start a transaction within a transaction" + ) + # The driver already holds an open implicit transaction - claim + # it, so writes stop committing automatically + self._explicit_transaction = True + return self.execute("BEGIN") def commit(self) -> None: """ Commit the current transaction. Does nothing if no transaction is open. """ + if self._autocommit_false: + self._explicit_transaction = False + self._commit_if_in_transaction() + return if self.conn.in_transaction: self.conn.execute("COMMIT") @@ -656,6 +741,11 @@ class Database: Roll back the current transaction, discarding its changes. Does nothing if no transaction is open. """ + if self._autocommit_false: + self._explicit_transaction = False + if self.conn.in_transaction: + self.conn.rollback() + return if self.conn.in_transaction: self.conn.execute("ROLLBACK") @@ -681,11 +771,25 @@ class Database: ``isolation_level`` would commit it as a side effect, silently breaking the caller's ability to roll back """ - if self.conn.in_transaction: + if self.in_transaction: raise TransactionError( "ensure_autocommit_on() cannot be used inside a transaction - " "changing isolation_level would commit the open transaction" ) + autocommit = getattr(self.conn, "autocommit", None) + if autocommit is True: + # Already in driver-level autocommit mode + yield + return + if autocommit is False: + # Setting autocommit = True commits the driver's implicit + # transaction - safe, because no user transaction is open + setattr(self.conn, "autocommit", True) + try: + yield + finally: + setattr(self.conn, "autocommit", False) + return old_isolation_level = self.conn.isolation_level try: self.conn.isolation_level = None @@ -848,11 +952,17 @@ class Database: # execute these without the savepoint guard used below. Some # adapters open an implicit transaction before comment-prefixed # PRAGMAs, so temporarily use driver autocommit when it is safe. - if self.conn.in_transaction: + if self.in_transaction: cursor = self.conn.execute(sql, *args) else: + fetch_eagerly = self._autocommit_false with self.ensure_autocommit_on(): cursor = self.conn.execute(sql, *args) + if fetch_eagerly and cursor.description is not None: + # Rows read lazily after autocommit is restored + # would hold a read transaction open + keys = dedupe_keys(d[0] for d in cursor.description) + return (dict(zip(keys, row)) for row in cursor.fetchall()) if cursor.description is None: raise ValueError(message) keys = dedupe_keys(d[0] for d in cursor.description) @@ -866,6 +976,16 @@ class Database: if cursor.description is None: raise ValueError(message) keys = dedupe_keys(d[0] for d in cursor.description) + if self._autocommit_false and not self._explicit_transaction: + # Fetch eagerly and commit the driver's implicit transaction. + # This commits any write (INSERT ... RETURNING) immediately, + # and releases the read snapshot which would otherwise stay + # open indefinitely, blocking writes from other connections + fetched = cursor.fetchall() + self.conn.execute('RELEASE "sqlite_utils_query"') + released = True + self._commit_if_in_transaction() + return (dict(zip(keys, row)) for row in fetched) try: self.conn.execute('RELEASE "sqlite_utils_query"') released = True @@ -878,6 +998,10 @@ class Database: fetched = cursor.fetchall() self.conn.execute('RELEASE "sqlite_utils_query"') released = True + if self._autocommit_false and not self._explicit_transaction: + # Releasing the savepoint cannot commit here, because the + # driver's implicit transaction encloses it + self._commit_if_in_transaction() return (dict(zip(keys, row)) for row in fetched) return (dict(zip(keys, row)) for row in cursor) finally: @@ -888,6 +1012,9 @@ class Database: # and there is nothing left to undo self.conn.execute('ROLLBACK TO "sqlite_utils_query"') self.conn.execute('RELEASE "sqlite_utils_query"') + if self._autocommit_false and not self._explicit_transaction: + # Also close the driver's implicit transaction + self.conn.rollback() def execute( self, sql: str, parameters: Optional[Union[Sequence, Dict[str, Any]]] = None @@ -906,6 +1033,8 @@ class Database: """ if self._tracer: self._tracer(sql, parameters) + if self._autocommit_false: + return self._execute_autocommit_false(sql, parameters) was_in_transaction = self.conn.in_transaction try: if parameters is not None: @@ -931,6 +1060,68 @@ class Database: self.conn.execute("COMMIT") return cursor + def _execute_autocommit_false( + self, sql: str, parameters: Optional[Union[Sequence, Dict[str, Any]]] = None + ) -> sqlite3.Cursor: + # execute() for Python 3.12+ autocommit=False connections, where the + # driver holds an implicit transaction open at all times. BEGIN, + # COMMIT and ROLLBACK are rerouted through begin()/commit()/rollback() + # so they behave identically to the other connection modes - the + # driver would otherwise reject BEGIN outright + keyword = _first_keyword(sql) + if keyword == "BEGIN": + self.begin() + return self.conn.cursor() + if keyword in ("COMMIT", "END"): + self.commit() + return self.conn.cursor() + if keyword == "ROLLBACK" and "TO" not in sql.upper().split(): + # A full ROLLBACK - but not ROLLBACK TO SAVEPOINT, which runs + # inside the transaction + self.rollback() + return self.conn.cursor() + if keyword in ("PRAGMA", "VACUUM") and not self._explicit_transaction: + # PRAGMA statements such as foreign_keys are silently ignored + # inside a transaction - and VACUUM refuses to run in one - but + # the driver's implicit transaction never closes. Run them in + # driver autocommit mode instead + with self.ensure_autocommit_on(): + if parameters is not None: + cursor = self.conn.execute(sql, parameters) + else: + cursor = self.conn.execute(sql) + if cursor.description is not None: + # Fetch eagerly - rows read lazily after autocommit is + # restored would hold a read transaction open + return cast( + sqlite3.Cursor, _PrefetchedCursor(cursor, cursor.fetchall()) + ) + return cursor + try: + if parameters is not None: + cursor = self.conn.execute(sql, parameters) + else: + cursor = self.conn.execute(sql) + except Exception: + if not self._explicit_transaction and self.conn.in_transaction: + # Discard whatever the failed statement left in the driver's + # implicit transaction, matching the other connection modes + self.conn.rollback() + raise + if not self._explicit_transaction: + if cursor.description is not None: + # A row-returning statement. Fetch eagerly and commit, so the + # driver's implicit read transaction - and the shared lock + # that blocks writes from other connections - is released + rows = cursor.fetchall() + self._commit_if_in_transaction() + return cast(sqlite3.Cursor, _PrefetchedCursor(cursor, rows)) + if keyword not in _TRANSACTION_CONTROL_KEYWORDS: + # No user transaction is open - commit the driver's implicit + # transaction, so writes behave identically across modes + self._commit_if_in_transaction() + return cursor + def executescript(self, sql: str) -> sqlite3.Cursor: """ Execute multiple SQL statements separated by ; and return the ``sqlite3.Cursor``. @@ -942,13 +1133,18 @@ class Database: return self._executescript(sql) def _executescript(self, sql: str) -> sqlite3.Cursor: - if self.conn.in_transaction: + if self.in_transaction: cursor = self.conn.cursor() # avoid sqlite3.executescript()'s implicit commit: for statement in _iter_complete_sql_statements(sql): cursor.execute(statement) return cursor - return self.conn.executescript(sql) + cursor = self.conn.executescript(sql) + if self._autocommit_false: + # executescript() does not commit in this mode - do it here, so + # scripts behave identically across connection modes + self._commit_if_in_transaction() + return cursor def table(self, table_name: str, **kwargs: Any) -> "Table": """ @@ -1188,7 +1384,7 @@ class Database: # Changing journal mode assigns conn.isolation_level, which commits # any open transaction as a side effect - breaking the rollback # guarantee of atomic() and of user-managed transactions - if self.conn.in_transaction: + if self.in_transaction: raise TransactionError( "{} cannot be used while a transaction is open".format(operation) ) @@ -1880,7 +2076,7 @@ class Database: for table, fks in by_table.items(): self.table(table).transform(add_foreign_keys=fks) - if not self.conn.in_transaction: + if not self.in_transaction: self.vacuum() def index_foreign_keys(self) -> None: @@ -1896,7 +2092,13 @@ class Database: def vacuum(self) -> None: "Run a SQLite ``VACUUM`` against the database." - self.execute("VACUUM;") + if self.in_transaction: + # Fails with OperationalError, as VACUUM always does inside a + # transaction - executed anyway for identical errors across modes + self.execute("VACUUM;") + else: + with self.ensure_autocommit_on(): + self.execute("VACUUM;") def analyze(self, name: Optional[str] = None) -> None: """ @@ -2554,7 +2756,7 @@ class Table(Queryable): pragma_foreign_keys_was_on = bool( self.db.execute("PRAGMA foreign_keys").fetchone()[0] ) - already_in_transaction = self.db.conn.in_transaction + already_in_transaction = self.db.in_transaction should_disable_foreign_keys = ( pragma_foreign_keys_was_on and not already_in_transaction ) diff --git a/tests/conftest.py b/tests/conftest.py index 728db7b..6e5f4e7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -18,6 +18,15 @@ def pytest_addoption(parser): "sqlite3.connect(autocommit=True) mode" ), ) + parser.addoption( + "--sqlite-autocommit-false", + action="store_true", + default=False, + help=( + "Run every test against connections created with the Python 3.12+ " + "sqlite3.connect(autocommit=False) mode" + ), + ) def pytest_configure(config): @@ -25,15 +34,22 @@ def pytest_configure(config): sys._called_from_test = True # type: ignore[attr-defined] - if config.getoption("--sqlite-autocommit"): + autocommit_true = config.getoption("--sqlite-autocommit") + autocommit_false = config.getoption("--sqlite-autocommit-false") + if autocommit_true and autocommit_false: + raise pytest.UsageError( + "--sqlite-autocommit and --sqlite-autocommit-false are mutually exclusive" + ) + if autocommit_true or autocommit_false: if sys.version_info < (3, 12): raise pytest.UsageError( - "--sqlite-autocommit requires Python 3.12 or higher" + "--sqlite-autocommit and --sqlite-autocommit-false require " + "Python 3.12 or higher" ) real_connect = sqlite3.connect def autocommit_connect(*args, **kwargs): - kwargs.setdefault("autocommit", True) + kwargs.setdefault("autocommit", autocommit_true) return real_connect(*args, **kwargs) sqlite3.connect = autocommit_connect @@ -81,5 +97,6 @@ def db_path(tmpdir): path = str(tmpdir / "test.db") db = sqlite3.connect(path) db.executescript(CREATE_TABLES) + db.commit() db.close() return path diff --git a/tests/test_analyze_tables.py b/tests/test_analyze_tables.py index a2ce585..885a397 100644 --- a/tests/test_analyze_tables.py +++ b/tests/test_analyze_tables.py @@ -135,8 +135,13 @@ def test_analyze_column(db_to_analyze, column, extra_kwargs, expected): def db_to_analyze_path(db_to_analyze, tmpdir): path = str(tmpdir / "test.db") db = sqlite3.connect(path) + if getattr(db, "autocommit", None) is False: + # iterdump scripts contain BEGIN TRANSACTION, which the driver's + # always-open implicit transaction would reject + db.autocommit = True sql = "\n".join(db_to_analyze.iterdump()) db.executescript(sql) + db.commit() db.close() return path diff --git a/tests/test_atomic.py b/tests/test_atomic.py index c3fd02f..d965ff0 100644 --- a/tests/test_atomic.py +++ b/tests/test_atomic.py @@ -121,7 +121,7 @@ def test_transform_does_not_commit_open_atomic_block(fresh_db): def test_transform_parent_table_with_foreign_keys_in_atomic(fresh_db): - fresh_db.conn.execute("PRAGMA foreign_keys=ON") + fresh_db.execute("PRAGMA foreign_keys=ON") fresh_db["authors"].insert({"id": 1, "name": "Tina"}, pk="id") fresh_db["books"].insert( {"id": 1, "title": "Book", "author_id": 1}, @@ -141,7 +141,7 @@ def test_transform_parent_table_with_foreign_keys_in_atomic(fresh_db): def test_transform_parent_table_with_foreign_keys_rolls_back(fresh_db): - fresh_db.conn.execute("PRAGMA foreign_keys=ON") + fresh_db.execute("PRAGMA foreign_keys=ON") fresh_db["authors"].insert({"id": 1, "name": "Tina"}, pk="id") fresh_db["books"].insert( {"id": 1, "title": "Book", "author_id": 1}, @@ -163,7 +163,7 @@ def test_transform_parent_table_with_foreign_keys_rolls_back(fresh_db): def test_transform_detects_foreign_key_check_violations(fresh_db): - fresh_db.conn.execute("PRAGMA foreign_keys=ON") + fresh_db.execute("PRAGMA foreign_keys=ON") fresh_db["authors"].insert({"id": 1, "name": "Tina"}, pk="id") fresh_db["books"].insert({"id": 1, "author_id": 2}, pk="id") @@ -180,7 +180,7 @@ def test_atomic_inside_manual_transaction_uses_savepoint(fresh_db): with fresh_db.atomic(): fresh_db["t"].insert({"id": 2}, pk="id") # Nothing is committed until the user's own transaction commits - assert fresh_db.conn.in_transaction + assert fresh_db.in_transaction fresh_db.rollback() assert [r["id"] for r in fresh_db["t"].rows] == [1] # And with a commit instead, the atomic block's writes persist @@ -197,9 +197,9 @@ def test_begin_commit_rollback(tmpdir): db["t"].insert({"id": 1}, pk="id") db.begin() db["t"].insert({"id": 2}, pk="id") - assert db.conn.in_transaction + assert db.in_transaction db.rollback() - assert not db.conn.in_transaction + assert not db.in_transaction assert [r["id"] for r in db["t"].rows] == [1] db.begin() db["t"].insert({"id": 3}, pk="id") @@ -220,7 +220,7 @@ def test_begin_inside_transaction_errors(fresh_db): def test_commit_and_rollback_without_transaction_are_noops(fresh_db): fresh_db.commit() fresh_db.rollback() - assert not fresh_db.conn.in_transaction + assert not fresh_db.in_transaction def test_execute_write_commits_immediately(tmpdir): @@ -229,7 +229,7 @@ def test_execute_write_commits_immediately(tmpdir): db["t"].insert({"id": 1}, pk="id") db.execute("insert into t (id) values (2)") # No implicit transaction is left open - assert not db.conn.in_transaction + assert not db.in_transaction # A completely separate connection sees the row straight away other = sqlite3.connect(path) assert other.execute("select count(*) from t").fetchone()[0] == 2 @@ -242,7 +242,7 @@ def test_execute_write_respects_explicit_transaction(fresh_db): fresh_db.begin() fresh_db.execute("insert into t (id) values (2)") # Still inside the explicit transaction - not committed - assert fresh_db.conn.in_transaction + assert fresh_db.in_transaction fresh_db.rollback() assert [r["id"] for r in fresh_db["t"].rows] == [1] @@ -252,7 +252,7 @@ def test_execute_comment_prefixed_begin_leaves_transaction_open(fresh_db): # out from under the caller fresh_db["t"].insert({"id": 1}, pk="id") fresh_db.execute("-- start a transaction\nbegin") - assert fresh_db.conn.in_transaction + assert fresh_db.in_transaction fresh_db.execute("insert into t (id) values (2)") fresh_db.rollback() assert [r["id"] for r in fresh_db["t"].rows] == [1] @@ -275,7 +275,7 @@ def test_execute_prefixed_begin_leaves_transaction_open(fresh_db, begin_sql): pytest.skip("This SQLite version rejects a leading byte order mark") fresh_db["t"].insert({"id": 1}, pk="id") fresh_db.execute(begin_sql) - assert fresh_db.conn.in_transaction + assert fresh_db.in_transaction fresh_db.execute("insert into t (id) values (2)") fresh_db.rollback() assert [r["id"] for r in fresh_db["t"].rows] == [1] @@ -289,7 +289,7 @@ def test_execute_failed_write_rolls_back_implicit_transaction(tmpdir): db["t"].insert({"id": 1}, pk="id") with pytest.raises(sqlite3.IntegrityError): db.execute("insert into t (id) values (1)") - assert not db.conn.in_transaction + assert not db.in_transaction # Subsequent writes commit as normal and survive closing the connection db["other"].insert({"id": 2}) db.close() @@ -306,7 +306,7 @@ def test_execute_failed_write_preserves_explicit_transaction(fresh_db): fresh_db.execute("insert into t (id) values (2)") with pytest.raises(sqlite3.IntegrityError): fresh_db.execute("insert into t (id) values (1)") - assert fresh_db.conn.in_transaction + assert fresh_db.in_transaction fresh_db.commit() assert [r["id"] for r in fresh_db["t"].rows] == [1, 2] @@ -332,7 +332,7 @@ def test_query_returning_commits_after_iteration(tmpdir): db["t"].insert({"id": 1}, pk="id") rows = list(db.query("insert into t (id) values (2) returning id")) assert rows == [{"id": 2}] - assert not db.conn.in_transaction + assert not db.in_transaction other = sqlite3.connect(path) assert other.execute("select count(*) from t").fetchone()[0] == 2 other.close() @@ -357,7 +357,7 @@ def test_atomic_preserves_error_from_transaction_destroying_trigger(fresh_db): with pytest.raises(sqlite3.IntegrityError, match="trigger says no"): with fresh_db.atomic(): fresh_db.execute("insert into t (v) values ('bad')") - assert not fresh_db.conn.in_transaction + assert not fresh_db.in_transaction def test_nested_atomic_preserves_error_from_transaction_destroying_trigger( @@ -371,7 +371,7 @@ def test_nested_atomic_preserves_error_from_transaction_destroying_trigger( with fresh_db.atomic(): with fresh_db.atomic(): fresh_db.execute("insert into t (v) values ('bad')") - assert not fresh_db.conn.in_transaction + assert not fresh_db.in_transaction def test_atomic_preserves_error_from_insert_or_rollback(fresh_db): @@ -379,4 +379,4 @@ def test_atomic_preserves_error_from_insert_or_rollback(fresh_db): with pytest.raises(sqlite3.IntegrityError): with fresh_db.atomic(): fresh_db.execute("insert or rollback into t (id) values (1)") - assert not fresh_db.conn.in_transaction + assert not fresh_db.in_transaction diff --git a/tests/test_cli.py b/tests/test_cli.py index a828c70..836cca5 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -2200,7 +2200,7 @@ def test_search_quote(tmpdir): def test_indexes(tmpdir): db_path = str(tmpdir / "test.db") db = Database(db_path) - db.conn.executescript(""" + db.executescript(""" create table Gosh (c1 text, c2 text, c3 text); create index Gosh_idx on Gosh(c2, c3 desc); """) @@ -2294,7 +2294,7 @@ def test_triggers(tmpdir, extra_args, expected): pk="id", ) db["counter"].insert({"count": 1}) - db.conn.execute(textwrap.dedent(""" + db.execute(textwrap.dedent(""" CREATE TRIGGER blah AFTER INSERT ON articles BEGIN UPDATE counter SET count = count + 1; diff --git a/tests/test_constructor.py b/tests/test_constructor.py index 470d089..13005d9 100644 --- a/tests/test_constructor.py +++ b/tests/test_constructor.py @@ -1,5 +1,4 @@ from sqlite_utils import Database -from sqlite_utils.db import TransactionError from sqlite_utils.utils import sqlite3 import pytest import sys @@ -65,23 +64,10 @@ def test_database_close(tmpdir, memory): sys.version_info < (3, 12), reason="sqlite3.connect(autocommit=) requires Python 3.12", ) -def test_autocommit_false_connections_are_rejected(tmpdir): - # autocommit=False keeps an implicit transaction open at all times, - # which breaks the explicit transaction handling used by every write - # method, so the constructor refuses these connections - conn = sqlite3.connect(str(tmpdir / "test.db"), autocommit=False) - with pytest.raises(TransactionError): - Database(conn) - conn.close() - - -@pytest.mark.skipif( - sys.version_info < (3, 12), - reason="sqlite3.connect(autocommit=) requires Python 3.12", -) -def test_autocommit_true_connection_writes_persist(tmpdir): +@pytest.mark.parametrize("autocommit", [True, False]) +def test_autocommit_connection_writes_persist(tmpdir, autocommit): path = str(tmpdir / "test.db") - conn = sqlite3.connect(path, autocommit=True) + conn = sqlite3.connect(path, autocommit=autocommit) db = Database(conn) db["t"].insert({"id": 1}, pk="id") db.execute("insert into t (id) values (2)") @@ -96,8 +82,9 @@ def test_autocommit_true_connection_writes_persist(tmpdir): sys.version_info < (3, 12), reason="sqlite3.connect(autocommit=) requires Python 3.12", ) -def test_autocommit_true_connection_transactions(tmpdir): - conn = sqlite3.connect(str(tmpdir / "test.db"), autocommit=True) +@pytest.mark.parametrize("autocommit", [True, False]) +def test_autocommit_connection_transactions(tmpdir, autocommit): + conn = sqlite3.connect(str(tmpdir / "test.db"), autocommit=autocommit) db = Database(conn) db["t"].insert({"id": 1}, pk="id") # atomic() rolls back on error @@ -124,8 +111,9 @@ def test_autocommit_true_connection_transactions(tmpdir): sys.version_info < (3, 12), reason="sqlite3.connect(autocommit=) requires Python 3.12", ) -def test_autocommit_true_connection_wal(tmpdir): - conn = sqlite3.connect(str(tmpdir / "test.db"), autocommit=True) +@pytest.mark.parametrize("autocommit", [True, False]) +def test_autocommit_connection_wal(tmpdir, autocommit): + conn = sqlite3.connect(str(tmpdir / "test.db"), autocommit=autocommit) db = Database(conn) db.enable_wal() assert db.journal_mode == "wal" diff --git a/tests/test_delete.py b/tests/test_delete.py index a2d93aa..28a08a9 100644 --- a/tests/test_delete.py +++ b/tests/test_delete.py @@ -42,7 +42,7 @@ def test_delete_where_commits(tmpdir): db["table"].delete_where("id > ?", [2]) # The connection must not be left inside an open transaction, # otherwise subsequent atomic() blocks never commit either - assert not db.conn.in_transaction + assert not db.in_transaction db["table"].insert({"id": 100}) db.close() db2 = sqlite_utils.Database(path) diff --git a/tests/test_fts.py b/tests/test_fts.py index 64ec645..d24b3e1 100644 --- a/tests/test_fts.py +++ b/tests/test_fts.py @@ -360,7 +360,7 @@ def test_optimize_and_rebuild_fts_commit(tmpdir, method): getattr(table, method)() # The connection must not be left inside an open transaction, # otherwise this and all subsequent writes are lost on close - assert not db.conn.in_transaction + assert not db.in_transaction table.insert(search_records[1]) db.close() db2 = Database(path) diff --git a/tests/test_query.py b/tests/test_query.py index 06847da..030cc72 100644 --- a/tests/test_query.py +++ b/tests/test_query.py @@ -24,14 +24,14 @@ def test_query_rejects_statements_that_return_no_rows(fresh_db): fresh_db.query("update dogs set name = 'Cleopaws'") assert "execute()" in str(ex.value) # The rejected update was rolled back, and no transaction is left open - assert not fresh_db.conn.in_transaction + assert not fresh_db.in_transaction assert [row["name"] for row in fresh_db["dogs"].rows] == ["Cleo"] def test_query_rejected_ddl_is_rolled_back(fresh_db): with pytest.raises(ValueError): fresh_db.query("create table dogs (id integer primary key)") - assert not fresh_db.conn.in_transaction + assert not fresh_db.in_transaction assert fresh_db.table_names() == [] @@ -42,7 +42,7 @@ def test_query_rejected_write_inside_transaction_is_rolled_back(fresh_db): with pytest.raises(ValueError): fresh_db.query("update dogs set name = 'Cleopaws'") # The transaction is still open and the earlier insert is intact - assert fresh_db.conn.in_transaction + assert fresh_db.in_transaction fresh_db.commit() assert [row["name"] for row in fresh_db["dogs"].rows] == ["Cleo", "Pancakes"] @@ -69,7 +69,7 @@ def test_query_rejects_transaction_control_and_vacuum(fresh_db, sql): with pytest.raises(ValueError) as ex: fresh_db.query(sql) assert "execute()" in str(ex.value) - assert not fresh_db.conn.in_transaction + assert not fresh_db.in_transaction def test_query_comment_prefixed_commit_does_not_commit_transaction(fresh_db): @@ -82,7 +82,7 @@ def test_query_comment_prefixed_commit_does_not_commit_transaction(fresh_db): with pytest.raises(ValueError): fresh_db.query("/* comment */ COMMIT") # The explicit transaction is still open and can still be rolled back - assert fresh_db.conn.in_transaction + assert fresh_db.in_transaction fresh_db.rollback() assert [row["name"] for row in fresh_db["dogs"].rows] == ["Cleo"] @@ -99,7 +99,7 @@ def test_query_prefixed_commit_does_not_commit_transaction(fresh_db, sql): with pytest.raises(ValueError): fresh_db.query(sql) # The explicit transaction is still open and can still be rolled back - assert fresh_db.conn.in_transaction + assert fresh_db.in_transaction fresh_db.rollback() assert [row["name"] for row in fresh_db["dogs"].rows] == ["Cleo"] @@ -107,7 +107,7 @@ def test_query_prefixed_commit_does_not_commit_transaction(fresh_db, sql): def test_query_error_leaves_no_transaction_open(fresh_db): with pytest.raises(sqlite3.OperationalError): fresh_db.query("select * from missing_table") - assert not fresh_db.conn.in_transaction + assert not fresh_db.in_transaction def test_query_pragma(tmpdir): @@ -152,7 +152,7 @@ def test_query_comment_prefixed_pragma_inside_transaction(fresh_db): assert list(fresh_db.query("-- check version\npragma user_version")) == [ {"user_version": 0} ] - assert fresh_db.conn.in_transaction + assert fresh_db.in_transaction fresh_db.rollback() @@ -209,7 +209,7 @@ def test_query_insert_returning_commits_without_iteration(tmpdir): db["dogs"].insert({"name": "Cleo"}) # Never iterate over the results db.query("insert into dogs (name) values ('Pancakes') returning name") - assert not db.conn.in_transaction + assert not db.in_transaction # A completely separate connection sees the new row straight away other = sqlite3.connect(path) assert other.execute("select count(*) from dogs").fetchone()[0] == 2 @@ -233,7 +233,7 @@ def test_query_insert_returning_partial_iteration_still_commits(tmpdir): ) ) assert row == {"name": "Pancakes"} - assert not db.conn.in_transaction + assert not db.in_transaction other = sqlite3.connect(path) assert other.execute("select count(*) from dogs").fetchone()[0] == 3 other.close() @@ -252,7 +252,7 @@ def test_query_insert_returning_respects_explicit_transaction(fresh_db): ) assert rows == [{"name": "Pancakes"}] # Still inside the explicit transaction - not committed - assert fresh_db.conn.in_transaction + assert fresh_db.in_transaction fresh_db.rollback() assert [row["name"] for row in fresh_db["dogs"].rows] == ["Cleo"] @@ -299,5 +299,5 @@ def test_query_preserves_error_from_transaction_destroying_trigger(fresh_db): """) with pytest.raises(sqlite3.IntegrityError, match="trigger says no"): fresh_db.query("insert into t (id, v) values (1, 'bad') returning id") - assert not fresh_db.conn.in_transaction + assert not fresh_db.in_transaction assert fresh_db.execute("select count(*) from t").fetchone()[0] == 0 diff --git a/tests/test_transform.py b/tests/test_transform.py index 71518be..31b1b84 100644 --- a/tests/test_transform.py +++ b/tests/test_transform.py @@ -109,7 +109,7 @@ def test_transform_sql_table_with_primary_key( dogs = fresh_db["dogs"] if use_pragma_foreign_keys: - fresh_db.conn.execute("PRAGMA foreign_keys=ON") + fresh_db.execute("PRAGMA foreign_keys=ON") dogs.insert({"id": 1, "name": "Cleo", "age": "5"}, pk="id") sql = dogs.transform_sql(**{**params, **{"tmp_suffix": "suffix"}}) assert sql == expected_sql @@ -182,7 +182,7 @@ def test_transform_sql_table_with_no_primary_key( dogs = fresh_db["dogs"] if use_pragma_foreign_keys: - fresh_db.conn.execute("PRAGMA foreign_keys=ON") + fresh_db.execute("PRAGMA foreign_keys=ON") dogs.insert({"id": 1, "name": "Cleo", "age": "5"}) sql = dogs.transform_sql(**{**params, **{"tmp_suffix": "suffix"}}) assert sql == expected_sql @@ -351,7 +351,7 @@ def test_transform_foreign_keys_survive_renamed_column( authors_db, use_pragma_foreign_keys ): if use_pragma_foreign_keys: - authors_db.conn.execute("PRAGMA foreign_keys=ON") + authors_db.execute("PRAGMA foreign_keys=ON") authors_db["books"].transform(rename={"author_id": "author_id_2"}) assert authors_db["books"].foreign_keys == [ ForeignKey( @@ -381,7 +381,7 @@ _CAVEAU = { @pytest.mark.parametrize("use_pragma_foreign_keys", [False, True]) def test_transform_drop_foreign_keys(fresh_db, use_pragma_foreign_keys): if use_pragma_foreign_keys: - fresh_db.conn.execute("PRAGMA foreign_keys=ON") + fresh_db.execute("PRAGMA foreign_keys=ON") # Create table with three foreign keys so we can drop two of them _add_country_city_continent(fresh_db) fresh_db["places"].insert( @@ -413,7 +413,7 @@ def test_transform_drop_foreign_keys(fresh_db, use_pragma_foreign_keys): def test_transform_verify_foreign_keys(fresh_db): - fresh_db.conn.execute("PRAGMA foreign_keys=ON") + fresh_db.execute("PRAGMA foreign_keys=ON") fresh_db["authors"].insert({"id": 3, "name": "Tina"}, pk="id") fresh_db["books"].insert( {"id": 1, "title": "Book", "author_id": 3}, pk="id", foreign_keys={"author_id"} diff --git a/tests/test_wal.py b/tests/test_wal.py index 2ddcf54..d7183e9 100644 --- a/tests/test_wal.py +++ b/tests/test_wal.py @@ -52,12 +52,17 @@ def test_disable_wal_inside_transaction_raises(db_path_tmpdir): def test_ensure_autocommit_on(db_path_tmpdir): db, path, tmpdir = db_path_tmpdir previous_isolation_level = db.conn.isolation_level - assert previous_isolation_level is not None + previous_autocommit = getattr(db.conn, "autocommit", None) with db.ensure_autocommit_on(): - # isolation_level of None means driver-level autocommit mode - assert db.conn.isolation_level is None + # Driver-level autocommit mode: raw writes on the connection do not + # open an implicit transaction, they commit immediately + db.conn.execute("create table t1 (id integer)") + db.conn.execute("insert into t1 values (1)") + assert not db.conn.in_transaction # Restored afterwards assert db.conn.isolation_level == previous_isolation_level + assert getattr(db.conn, "autocommit", None) == previous_autocommit + assert db.execute("select count(*) from t1").fetchone()[0] == 1 def test_enable_wal_noop_inside_transaction_is_allowed(db_path_tmpdir): @@ -83,6 +88,6 @@ def test_ensure_autocommit_on_inside_transaction_raises(db_path_tmpdir): with db.ensure_autocommit_on(): pass # The transaction is still open and can still be rolled back - assert db.conn.in_transaction + assert db.in_transaction db.rollback() assert [r["id"] for r in db["test"].rows] == [1]