From 8f3c2111fbc96493a052189bdd961e6f2cb16153 Mon Sep 17 00:00:00 2001 From: Gyeongjae Choi Date: Fri, 14 Aug 2026 14:53:26 +0900 Subject: [PATCH] fix(django-cf): return a fresh new cursor for every cursor call --- .../django-cf/django_cf/db/base_engine.py | 103 +++++++++++------- .../in_worker/worker/src/test_base_engine.py | 102 +++++++++++++---- .../in_worker/worker/src/test_d1_backend.py | 85 ++++++++++++++- 3 files changed, 225 insertions(+), 65 deletions(-) diff --git a/packages/django-cf/django_cf/db/base_engine.py b/packages/django-cf/django_cf/db/base_engine.py index 940114e..b75f5fc 100644 --- a/packages/django-cf/django_cf/db/base_engine.py +++ b/packages/django-cf/django_cf/db/base_engine.py @@ -277,63 +277,41 @@ def from_object( return instance -class CFDatabase: - def __init__(self, database_wrapper): - self.databaseWrapper = database_wrapper - - DataError = DataError - - OperationalError = OperationalError - - IntegrityError = IntegrityError - - InternalError = InternalError - - ProgrammingError = ProgrammingError - - NotSupportedError = NotSupportedError - DatabaseError = DatabaseError - InterfaceError = InterfaceError - Error = Error - - _defer_foreign_keys = False +class CFCursor: + def __init__(self, database): + self.database = database + self.lastResult: CFResult | None = None - lastResult: CFResult = None + @property + def _defer_foreign_keys(self): + return self.database._defer_foreign_keys def defer_foreign_keys(self, state): - self._defer_foreign_keys = state - - @staticmethod - def connect(binding): - return CFDatabase(binding) - - def cursor(self): - return self - - def commit(self): - return # No commits allowed - - def rollback(self): - return # No commits allowed + self.database.defer_foreign_keys(state) def fetchone(self): - return self.lastResult.fetchone() + result = self.lastResult + return result.fetchone() def fetchall(self): - return self.lastResult.fetchall() + result = self.lastResult + return result.fetchall() def fetchmany(self, size=1): - return self.lastResult.fetchmany(size) + result = self.lastResult + return result.fetchmany(size) @property def lastrowid(self): - return self.lastResult.lastrowid + result = self.lastResult + return result.lastrowid @property def rowcount(self): - return self.lastResult.rowcount + result = self.lastResult + return result.rowcount - def execute(self, query, params=None) -> None: + def execute(self, query, params=None): from decimal import Decimal # Transform django_date_trunc function calls to SQLite equivalents @@ -353,7 +331,7 @@ def execute(self, query, params=None) -> None: params = tuple(newParams) - self.lastResult = self.databaseWrapper.run_query(query, params) + self.lastResult = self.database.databaseWrapper.run_query(query, params) return self @@ -361,6 +339,47 @@ def close(self): return +class CFDatabase: + def __init__(self, database_wrapper): + self.databaseWrapper = database_wrapper + + DataError = DataError + + OperationalError = OperationalError + + IntegrityError = IntegrityError + + InternalError = InternalError + + ProgrammingError = ProgrammingError + + NotSupportedError = NotSupportedError + DatabaseError = DatabaseError + InterfaceError = InterfaceError + Error = Error + + _defer_foreign_keys = False + + def defer_foreign_keys(self, state): + self._defer_foreign_keys = state + + @staticmethod + def connect(binding): + return CFDatabase(binding) + + def cursor(self): + return CFCursor(self) + + def commit(self): + return # No commits allowed + + def rollback(self): + return # No commits allowed + + def close(self): + return + + def is_read_only_query(query: str) -> bool: parsed = sqlparse.parse(query.strip()) diff --git a/packages/django-cf/tests/in_worker/worker/src/test_base_engine.py b/packages/django-cf/tests/in_worker/worker/src/test_base_engine.py index 5c2d24c..cb01bbb 100644 --- a/packages/django-cf/tests/in_worker/worker/src/test_base_engine.py +++ b/packages/django-cf/tests/in_worker/worker/src/test_base_engine.py @@ -327,11 +327,18 @@ def test_connect(self): assert db.databaseWrapper == mock_wrapper - def test_cursor_returns_self(self): - from django_cf.db.base_engine import CFDatabase + def test_cursor_returns_fresh_cursor_each_call(self): + from django_cf.db.base_engine import CFCursor, CFDatabase db = CFDatabase(MagicMock()) - assert db.cursor() is db + first_cursor = db.cursor() + second_cursor = db.cursor() + + assert isinstance(first_cursor, CFCursor) + assert isinstance(second_cursor, CFCursor) + assert first_cursor is not second_cursor + assert first_cursor.database is db + assert second_cursor.database is db def test_commit_does_nothing(self): from django_cf.db.base_engine import CFDatabase @@ -357,14 +364,29 @@ def test_defer_foreign_keys(self): db.defer_foreign_keys(False) assert db._defer_foreign_keys is False + def test_cursor_defer_foreign_keys_proxies_database_state(self): + from django_cf.db.base_engine import CFDatabase + + db = CFDatabase(MagicMock()) + first_cursor = db.cursor() + second_cursor = db.cursor() + + first_cursor.defer_foreign_keys(True) + assert db._defer_foreign_keys is True + assert second_cursor._defer_foreign_keys is True + + second_cursor.defer_foreign_keys(False) + assert db._defer_foreign_keys is False + assert first_cursor._defer_foreign_keys is False + def test_execute_converts_boolean_true(self): from django_cf.db.base_engine import CFDatabase, CFResult mock_wrapper = MagicMock() mock_wrapper.run_query.return_value = CFResult([]) - db = CFDatabase(mock_wrapper) + cursor = CFDatabase(mock_wrapper).cursor() - db.execute("INSERT INTO test VALUES (%s)", (True,)) + cursor.execute("INSERT INTO test VALUES (%s)", (True,)) assert mock_wrapper.run_query.call_args[0][1] == (1,) @@ -373,9 +395,9 @@ def test_execute_converts_boolean_false(self): mock_wrapper = MagicMock() mock_wrapper.run_query.return_value = CFResult([]) - db = CFDatabase(mock_wrapper) + cursor = CFDatabase(mock_wrapper).cursor() - db.execute("INSERT INTO test VALUES (%s)", (False,)) + cursor.execute("INSERT INTO test VALUES (%s)", (False,)) assert mock_wrapper.run_query.call_args[0][1] == (0,) @@ -384,9 +406,9 @@ def test_execute_converts_decimal_to_string(self): mock_wrapper = MagicMock() mock_wrapper.run_query.return_value = CFResult([]) - db = CFDatabase(mock_wrapper) + cursor = CFDatabase(mock_wrapper).cursor() - db.execute("INSERT INTO test VALUES (%s)", (Decimal("10.5"),)) + cursor.execute("INSERT INTO test VALUES (%s)", (Decimal("10.5"),)) assert mock_wrapper.run_query.call_args[0][1] == ("10.5",) @@ -395,33 +417,57 @@ def test_execute_no_params(self): mock_wrapper = MagicMock() mock_wrapper.run_query.return_value = CFResult([]) - db = CFDatabase(mock_wrapper) + cursor = CFDatabase(mock_wrapper).cursor() - db.execute("SELECT * FROM test") + cursor.execute("SELECT * FROM test") assert mock_wrapper.run_query.call_args[0] == ("SELECT * FROM test", None) + def test_execute_returns_cursor(self): + from django_cf.db.base_engine import CFDatabase, CFResult + + mock_wrapper = MagicMock() + mock_wrapper.run_query.return_value = CFResult([]) + cursor = CFDatabase(mock_wrapper).cursor() + + assert cursor.execute("SELECT * FROM test") is cursor + def test_fetchone_delegates_to_result(self): from django_cf.db.base_engine import CFDatabase, CFResult mock_wrapper = MagicMock() mock_wrapper.run_query.return_value = CFResult([(1, "test")]) - db = CFDatabase(mock_wrapper) + cursor = CFDatabase(mock_wrapper).cursor() - db.execute("SELECT * FROM test") + cursor.execute("SELECT * FROM test") - assert db.fetchone() == (1, "test") + assert cursor.fetchone() == (1, "test") def test_fetchall_delegates_to_result(self): from django_cf.db.base_engine import CFDatabase, CFResult mock_wrapper = MagicMock() mock_wrapper.run_query.return_value = CFResult([(1, "a"), (2, "b")]) + cursor = CFDatabase(mock_wrapper).cursor() + + cursor.execute("SELECT * FROM test") + + assert len(cursor.fetchall()) == 2 + + def test_cursors_keep_independent_results(self): + from django_cf.db.base_engine import CFDatabase, CFResult + + mock_wrapper = MagicMock() + mock_wrapper.run_query.side_effect = [CFResult([(1,), (2,)]), CFResult([(9,)])] db = CFDatabase(mock_wrapper) + outer_cursor = db.cursor() + inner_cursor = db.cursor() - db.execute("SELECT * FROM test") + outer_cursor.execute("SELECT * FROM outer_table") + inner_cursor.execute("SELECT * FROM inner_table") - assert len(db.fetchall()) == 2 + assert inner_cursor.fetchone() == (9,) + assert sorted(outer_cursor.fetchall()) == [(1,), (2,)] def test_lastrowid_property(self): from django_cf.db.base_engine import CFDatabase, CFResult @@ -430,11 +476,11 @@ def test_lastrowid_property(self): mock_result = CFResult([]) mock_result.set_lastrowid(42) mock_wrapper.run_query.return_value = mock_result - db = CFDatabase(mock_wrapper) + cursor = CFDatabase(mock_wrapper).cursor() - db.execute("INSERT INTO test VALUES (1)") + cursor.execute("INSERT INTO test VALUES (1)") - assert db.lastrowid == 42 + assert cursor.lastrowid == 42 def test_rowcount_property(self): from django_cf.db.base_engine import CFDatabase, CFResult @@ -443,11 +489,23 @@ def test_rowcount_property(self): mock_result = CFResult([]) mock_result.set_rowcount(5) mock_wrapper.run_query.return_value = mock_result - db = CFDatabase(mock_wrapper) + cursor = CFDatabase(mock_wrapper).cursor() + + cursor.execute('UPDATE test SET name = "new"') + + assert cursor.rowcount == 5 + + def test_cursor_close_is_noop_and_keeps_result(self): + from django_cf.db.base_engine import CFDatabase, CFResult + + mock_wrapper = MagicMock() + mock_wrapper.run_query.return_value = CFResult([(1,), (2,)]) + cursor = CFDatabase(mock_wrapper).cursor() - db.execute('UPDATE test SET name = "new"') + cursor.execute("SELECT * FROM test") - assert db.rowcount == 5 + assert cursor.close() is None + assert sorted(cursor.fetchall()) == [(1,), (2,)] class TestCFDatabaseFeatures: diff --git a/packages/django-cf/tests/in_worker/worker/src/test_d1_backend.py b/packages/django-cf/tests/in_worker/worker/src/test_d1_backend.py index ff088c5..4b1b5ee 100644 --- a/packages/django-cf/tests/in_worker/worker/src/test_d1_backend.py +++ b/packages/django-cf/tests/in_worker/worker/src/test_d1_backend.py @@ -2,10 +2,58 @@ # pyright: reportMissingImports=false +import asyncio +from contextlib import contextmanager + import pytest -from django.db import connections +from django.db import connections, models D1_BACKEND = connections["d1"] +D1_PARENT_TABLE = "_django_cf_d1_cursor_parents" +D1_CHILD_TABLE = "_django_cf_d1_cursor_children" + + +class D1CursorParent(models.Model): + name = models.CharField(max_length=64) + + class Meta: + app_label = "django_cf_in_worker" + db_table = D1_PARENT_TABLE + managed = False + + +class D1CursorChild(models.Model): + parent = models.ForeignKey(D1CursorParent, on_delete=models.CASCADE) + value = models.CharField(max_length=64) + + class Meta: + app_label = "django_cf_in_worker" + db_table = D1_CHILD_TABLE + managed = False + + +def _drop_cursor_tables(): + D1_BACKEND.run_query(f"DROP TABLE IF EXISTS {D1_CHILD_TABLE}") + D1_BACKEND.run_query(f"DROP TABLE IF EXISTS {D1_PARENT_TABLE}") + + +@contextmanager +def _cursor_tables(): + _drop_cursor_tables() + try: + D1_BACKEND.run_query( + f"CREATE TABLE {D1_PARENT_TABLE} (id INTEGER PRIMARY KEY AUTOINCREMENT, name TEXT NOT NULL)" + ) + D1_BACKEND.run_query( + f"CREATE TABLE {D1_CHILD_TABLE} (" + "id INTEGER PRIMARY KEY AUTOINCREMENT, " + "parent_id INTEGER NOT NULL REFERENCES " + f"{D1_PARENT_TABLE}(id), " + "value TEXT NOT NULL)" + ) + yield + finally: + _drop_cursor_tables() class TestD1DatabaseWrapperProcessQuery: @@ -160,6 +208,41 @@ def test_write_query_returns_meta(self): assert result.lastrowid == 1 wrapper.run_query(f"DROP TABLE {table}") + @pytest.mark.asyncio + async def test_concurrent_cursors_on_one_wrapper_keep_independent_results(self): + wrapper = D1_BACKEND + + async def execute(value): + cursor = wrapper.cursor() + cursor.execute("SELECT %s", [value]) + return cursor + + alpha_cursor, beta_cursor = await asyncio.gather( + execute("alpha"), execute("beta") + ) + + assert alpha_cursor.fetchone() == ("alpha",) + assert beta_cursor.fetchone() == ("beta",) + + def test_iterator_chunk_size_one_survives_nested_foreign_key_queries(self): + """Make sure nested child-parent queryset returns a proper result""" + with _cursor_tables(): + first_parent = D1CursorParent.objects.using("d1").create(name="alpha") + second_parent = D1CursorParent.objects.using("d1").create(name="beta") + D1CursorChild.objects.using("d1").create( + parent_id=first_parent.pk, value="one" + ) + D1CursorChild.objects.using("d1").create( + parent_id=second_parent.pk, value="two" + ) + + seen = [] + queryset = D1CursorChild.objects.using("d1").order_by("id") + for child in queryset.iterator(chunk_size=1): + seen.append((child.value, child.parent.name)) + + assert sorted(seen) == [("one", "alpha"), ("two", "beta")] + class TestD1ParameterHandling: def test_empty_params_list(self):