From 5cec9c9faa523b116d1bfdc7d58d4031da4cceec Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 9 Jul 2026 06:03:42 +0000 Subject: [PATCH] Stop running sqlite-utils plugins on Datasette connections Wrapping a connection in sqlite_utils.Database() runs sqlite-utils plugins' prepare_connection hooks against it by default. Datasette's write API views and introspection helpers now pass execute_plugins=False (matching what utils/internal_db.py already did), so third-party sqlite-utils plugins no longer touch Datasette's connections. Also apply PRAGMA recursive_triggers=on in Datasette._prepare_connection so every connection gets consistent trigger semantics - previously only the write connection got it, as a side effect of the first sqlite-utils based write. Refs #2831 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01N76afGMhBRQk528VF1LTpR --- datasette/app.py | 4 +++ datasette/database.py | 2 +- datasette/views/row.py | 6 +++-- datasette/views/table.py | 4 +-- datasette/views/table_create_alter.py | 10 +++++--- docs/changelog.rst | 1 + tests/test_api_write.py | 35 +++++++++++++++++++++++++++ tests/test_internals_database.py | 21 ++++++++++++++++ 8 files changed, 75 insertions(+), 8 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index 4ba5d20f..987c77df 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -1565,6 +1565,10 @@ class Datasette: conn.execute("SELECT load_extension(?)", [extension]) if self.setting("cache_size_kb"): conn.execute(f"PRAGMA cache_size=-{self.setting('cache_size_kb')}") + # Consistent trigger semantics on every connection - sqlite-utils + # would otherwise enable this on just the write connection, as a + # side effect of the first sqlite-utils based write + conn.execute("PRAGMA recursive_triggers=on") # pylint: disable=no-member if database != INTERNAL_DB_NAME: pm.hook.prepare_connection(conn=conn, database=database, datasette=self) diff --git a/datasette/database.py b/datasette/database.py index 82522e6a..6c1b3a9e 100644 --- a/datasette/database.py +++ b/datasette/database.py @@ -757,7 +757,7 @@ class Database: def column_details(conn): # Returns {column_name: (type, is_unique)} - db = sqlite_utils.Database(conn) + db = sqlite_utils.Database(conn, execute_plugins=False) columns = db[table].columns_dict indexes = db[table].indexes details = {} diff --git a/datasette/views/row.py b/datasette/views/row.py index c90a3bbe..feb67eb5 100644 --- a/datasette/views/row.py +++ b/datasette/views/row.py @@ -749,7 +749,9 @@ class RowDeleteView(BaseView): # Delete table def delete_row(conn): - sqlite_utils.Database(conn)[resolved.table].delete(resolved.pk_values) + sqlite_utils.Database(conn, execute_plugins=False)[resolved.table].delete( + resolved.pk_values + ) try: await resolved.db.execute_write_fn(delete_row, request=request) @@ -830,7 +832,7 @@ class RowUpdateView(BaseView): return Response.error(["Permission denied for alter-table"], 403) def update_row(conn): - sqlite_utils.Database(conn)[resolved.table].update( + sqlite_utils.Database(conn, execute_plugins=False)[resolved.table].update( resolved.pk_values, update, alter=alter ) diff --git a/datasette/views/table.py b/datasette/views/table.py index f7edd744..35afd223 100644 --- a/datasette/views/table.py +++ b/datasette/views/table.py @@ -1143,7 +1143,7 @@ class TableInsertView(BaseView): row_pk_values_for_later = [tuple(row[pk] for pk in pks) for row in rows] def insert_or_upsert_rows(conn): - table = sqlite_utils.Database(conn)[table_name] + table = sqlite_utils.Database(conn, execute_plugins=False)[table_name] kwargs = {} if upsert: kwargs = { @@ -1407,7 +1407,7 @@ class TableDropView(BaseView): # Drop table def drop_table(conn): - sqlite_utils.Database(conn)[table_name].drop() + sqlite_utils.Database(conn, execute_plugins=False)[table_name].drop() await db.execute_write_fn(drop_table, request=request) await self.ds.track_event( diff --git a/datasette/views/table_create_alter.py b/datasette/views/table_create_alter.py index f3e1779f..35f7ab86 100644 --- a/datasette/views/table_create_alter.py +++ b/datasette/views/table_create_alter.py @@ -889,11 +889,13 @@ class TableCreateView(BaseView): initial_schema = None if table_exists: initial_schema = await db.execute_fn( - lambda conn: sqlite_utils.Database(conn)[table_name].schema + lambda conn: sqlite_utils.Database(conn, execute_plugins=False)[ + table_name + ].schema ) def create_table(conn): - db_for_write = sqlite_utils.Database(conn) + db_for_write = sqlite_utils.Database(conn, execute_plugins=False) table = db_for_write[table_name] if rows: table.insert_all( @@ -1187,7 +1189,9 @@ class TableAlterView(BaseView): before_schema = _table_schema_from_conn(conn, table_name) def apply_operations(operation_conn): - db_for_write = sqlite_utils.Database(operation_conn) + db_for_write = sqlite_utils.Database( + operation_conn, execute_plugins=False + ) table = db_for_write[table_name] current_table_name = table_name diff --git a/docs/changelog.rst b/docs/changelog.rst index 8cb72005..caa1050f 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -14,6 +14,7 @@ Unreleased - ``await db.execute_write_script()`` is now transactional, matching its documentation: if any statement in the script fails, none of its statements are applied. Scripts containing statements that cannot run inside a transaction, or that manage transactions themselves, fall back to the previous ``conn.executescript()`` autocommit behavior. (:issue:`2831`) - The JSON write API is now atomic per request: ``/db/-/create`` with initial rows, multi-operation ``/db/table/-/alter`` calls and inserts using ``"return": true`` now either fully apply or roll back entirely if any part fails. Previously a failure part way through could leave earlier writes from the same request permanently committed. (:issue:`2831`) - Rebuilding the internal database catalog for a database is now a single atomic write. Previously the rebuild used six separate transactions, so queries against the internal database could observe a database with missing catalog rows while a rebuild was in progress. (:issue:`2831`) +- sqlite-utils plugins no longer have their ``prepare_connection()`` hooks executed against Datasette's database connections - use Datasette's own :ref:`prepare_connection() ` plugin hook to customize connections. ``PRAGMA recursive_triggers=on`` is now applied consistently to every connection Datasette opens - previously it was enabled just on the write connection, as a side effect of the first sqlite-utils based write. (:issue:`2831`) .. _v1_0_a36: diff --git a/tests/test_api_write.py b/tests/test_api_write.py index 691f7ac9..4218712e 100644 --- a/tests/test_api_write.py +++ b/tests/test_api_write.py @@ -2782,3 +2782,38 @@ async def test_insert_with_return_failing_row_is_atomic(ds_write): await ds_write.get_database("data").execute("select count(*) from docs") ).single_value() assert count == 0 + + +@pytest.mark.asyncio +async def test_write_api_does_not_run_sqlite_utils_plugins(ds_write): + # https://github.com/simonw/datasette/issues/2831 + # sqlite-utils plugins should not have their prepare_connection hooks + # executed against Datasette's connections + import sqlite_utils.plugins + from sqlite_utils import hookimpl + + prepared = [] + + class TrackingPlugin: + @hookimpl + def prepare_connection(self, conn): + prepared.append(conn) + + sqlite_utils.plugins.pm.register(TrackingPlugin(), name="datasette-test-tracking") + try: + token = write_token(ds_write) + response = await ds_write.client.post( + "/data/docs/-/insert", + json={"rows": [{"id": 1, "title": "one"}]}, + headers=_headers(token), + ) + assert response.status_code == 201 + response = await ds_write.client.post( + "/data/docs/1/-/update", + json={"update": {"title": "two"}}, + headers=_headers(token), + ) + assert response.status_code == 200 + assert prepared == [] + finally: + sqlite_utils.plugins.pm.unregister(name="datasette-test-tracking") diff --git a/tests/test_internals_database.py b/tests/test_internals_database.py index 74818df9..fb1adcba 100644 --- a/tests/test_internals_database.py +++ b/tests/test_internals_database.py @@ -1302,3 +1302,24 @@ async def test_database_close_is_idempotent(tmpdir): # Second call should be a no-op, not raise db.close() ds._internal_database.close() + + +@pytest.mark.asyncio +async def test_recursive_triggers_enabled_on_all_connections(tmp_path): + # https://github.com/simonw/datasette/issues/2831 + # Previously recursive_triggers was only enabled on the write connection, + # and only as a side effect of the first sqlite-utils based write - so + # trigger semantics could differ between connections + path = str(tmp_path / "test.db") + sqlite3.connect(path).close() + datasette = Datasette([path]) + db = datasette.get_database("test") + write_value = await db.execute_write_fn( + lambda conn: conn.execute("PRAGMA recursive_triggers").fetchone()[0], + transaction=False, + ) + read_value = await db.execute_fn( + lambda conn: conn.execute("PRAGMA recursive_triggers").fetchone()[0] + ) + assert write_value == 1 + assert read_value == 1