diff options
| -rw-r--r-- | django/db/backends/sqlite3/base.py | 12 | ||||
| -rw-r--r-- | django/db/backends/sqlite3/schema.py | 11 | ||||
| -rw-r--r-- | docs/releases/2.0.10.txt | 3 | ||||
| -rw-r--r-- | tests/backends/sqlite/tests.py | 64 | ||||
| -rw-r--r-- | tests/indexes/tests.py | 24 | ||||
| -rw-r--r-- | tests/model_options/test_tablespaces.py | 7 | ||||
| -rw-r--r-- | tests/test_runner/tests.py | 4 |
7 files changed, 86 insertions, 39 deletions
diff --git a/django/db/backends/sqlite3/base.py b/django/db/backends/sqlite3/base.py index eabcdfabe5..fd1d85b908 100644 --- a/django/db/backends/sqlite3/base.py +++ b/django/db/backends/sqlite3/base.py @@ -215,11 +215,13 @@ class DatabaseWrapper(BaseDatabaseWrapper): self.connection.isolation_level = level def disable_constraint_checking(self): - if self.in_atomic_block: - # sqlite3 cannot disable constraint checking inside a transaction. - return False - self.cursor().execute('PRAGMA foreign_keys = OFF') - return True + with self.cursor() as cursor: + cursor.execute('PRAGMA foreign_keys = OFF') + # Foreign key constraints cannot be turned off while in a multi- + # statement transaction. Fetch the current state of the pragma + # to determine if constraints are effectively disabled. + enabled = cursor.execute('PRAGMA foreign_keys').fetchone()[0] + return not bool(enabled) def enable_constraint_checking(self): self.cursor().execute('PRAGMA foreign_keys = ON') diff --git a/django/db/backends/sqlite3/schema.py b/django/db/backends/sqlite3/schema.py index bd2278b83b..29eaa033f4 100644 --- a/django/db/backends/sqlite3/schema.py +++ b/django/db/backends/sqlite3/schema.py @@ -20,8 +20,15 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor): def __enter__(self): # Some SQLite schema alterations need foreign key constraints to be - # disabled. Enforce it here for the duration of the transaction. - self.connection.disable_constraint_checking() + # disabled. Enforce it here for the duration of the schema edition. + if not self.connection.disable_constraint_checking(): + raise NotSupportedError( + 'SQLite schema editor cannot be used while foreign key ' + 'constraint checks are enabled. Make sure to disable them ' + 'before entering a transaction.atomic() context because ' + 'SQLite3 does not support disabling them in the middle of ' + 'a multi-statement transaction.' + ) self.connection.cursor().execute('PRAGMA legacy_alter_table = ON') return super().__enter__() diff --git a/docs/releases/2.0.10.txt b/docs/releases/2.0.10.txt index ece2521867..18901490e0 100644 --- a/docs/releases/2.0.10.txt +++ b/docs/releases/2.0.10.txt @@ -16,3 +16,6 @@ Bugfixes * Fixed a schema corruption issue on SQLite 3.26+. You might have to drop and rebuild your SQLite database if you applied a migration while using an older version of Django with SQLite 3.26 or later (:ticket:`29182`). + +* Prevented SQLite schema alterations while foreign key checks are enabled to + avoid the possibility of schema corruption (:ticket:`30023`). diff --git a/tests/backends/sqlite/tests.py b/tests/backends/sqlite/tests.py index 7168280e1f..e95280ede7 100644 --- a/tests/backends/sqlite/tests.py +++ b/tests/backends/sqlite/tests.py @@ -3,7 +3,7 @@ import threading import unittest from django.core.exceptions import ImproperlyConfigured -from django.db import connection +from django.db import connection, transaction from django.db.models import Avg, StdDev, Sum, Variance from django.db.models.fields import CharField from django.db.utils import NotSupportedError @@ -19,22 +19,6 @@ from ..models import Author, Item, Object, Square class Tests(TestCase): longMessage = True - def test_autoincrement(self): - """ - auto_increment fields are created with the AUTOINCREMENT keyword - in order to be monotonically increasing (#10164). - """ - with connection.schema_editor(collect_sql=True) as editor: - editor.create_model(Square) - statements = editor.collected_sql - match = re.search('"id" ([^,]+),', statements[0]) - self.assertIsNotNone(match) - self.assertEqual( - 'integer NOT NULL PRIMARY KEY AUTOINCREMENT', - match.group(1), - 'Wrong SQL used to create an auto-increment column on SQLite' - ) - def test_aggregation(self): """ Raise NotImplementedError when aggregating on date/time fields (#19360). @@ -80,6 +64,52 @@ class SchemaTests(TransactionTestCase): available_apps = ['backends'] + def test_autoincrement(self): + """ + auto_increment fields are created with the AUTOINCREMENT keyword + in order to be monotonically increasing (#10164). + """ + with connection.schema_editor(collect_sql=True) as editor: + editor.create_model(Square) + statements = editor.collected_sql + match = re.search('"id" ([^,]+),', statements[0]) + self.assertIsNotNone(match) + self.assertEqual( + 'integer NOT NULL PRIMARY KEY AUTOINCREMENT', + match.group(1), + 'Wrong SQL used to create an auto-increment column on SQLite' + ) + + def test_disable_constraint_checking_failure_disallowed(self): + """ + SQLite schema editor is not usable within an outer transaction if + foreign key constraint checks are not disabled beforehand. + """ + msg = ( + 'SQLite schema editor cannot be used while foreign key ' + 'constraint checks are enabled. Make sure to disable them ' + 'before entering a transaction.atomic() context because ' + 'SQLite3 does not support disabling them in the middle of ' + 'a multi-statement transaction.' + ) + with self.assertRaisesMessage(NotSupportedError, msg): + with transaction.atomic(), connection.schema_editor(atomic=True): + pass + + def test_constraint_checks_disabled_atomic_allowed(self): + """ + SQLite3 schema editor is usable within an outer transaction as long as + foreign key constraints checks are disabled beforehand. + """ + def constraint_checks_enabled(): + with connection.cursor() as cursor: + return bool(cursor.execute('PRAGMA foreign_keys').fetchone()[0]) + with connection.constraint_checks_disabled(), transaction.atomic(): + with connection.schema_editor(atomic=True): + self.assertFalse(constraint_checks_enabled()) + self.assertFalse(constraint_checks_enabled()) + self.assertTrue(constraint_checks_enabled()) + def test_field_rename_inside_atomic_block(self): """ NotImplementedError is raised when a model field rename is attempted diff --git a/tests/indexes/tests.py b/tests/indexes/tests.py index ee2cbd1564..75baebf04a 100644 --- a/tests/indexes/tests.py +++ b/tests/indexes/tests.py @@ -17,12 +17,12 @@ class SchemaIndexesTests(TestCase): """ Index names should be deterministic. """ - with connection.schema_editor() as editor: - index_name = editor._create_index_name( - table_name=Article._meta.db_table, - column_names=("c1",), - suffix="123", - ) + editor = connection.schema_editor() + index_name = editor._create_index_name( + table_name=Article._meta.db_table, + column_names=("c1",), + suffix="123", + ) self.assertEqual(index_name, "indexes_article_c1_a52bd80b123") def test_index_name(self): @@ -33,12 +33,12 @@ class SchemaIndexesTests(TestCase): * Include a deterministic hash. """ long_name = 'l%sng' % ('o' * 100) - with connection.schema_editor() as editor: - index_name = editor._create_index_name( - table_name=Article._meta.db_table, - column_names=('c1', 'c2', long_name), - suffix='ix', - ) + editor = connection.schema_editor() + index_name = editor._create_index_name( + table_name=Article._meta.db_table, + column_names=('c1', 'c2', long_name), + suffix='ix', + ) expected = { 'mysql': 'indexes_article_c1_c2_looooooooooooooooooo_255179b2ix', 'oracle': 'indexes_a_c1_c2_loo_255179b2ix', diff --git a/tests/model_options/test_tablespaces.py b/tests/model_options/test_tablespaces.py index 79b0a8bb75..8502d8790f 100644 --- a/tests/model_options/test_tablespaces.py +++ b/tests/model_options/test_tablespaces.py @@ -1,7 +1,9 @@ from django.apps import apps from django.conf import settings from django.db import connection -from django.test import TestCase, skipIfDBFeature, skipUnlessDBFeature +from django.test import ( + TransactionTestCase, skipIfDBFeature, skipUnlessDBFeature, +) from .models.tablespaces import ( Article, ArticleRef, Authors, Reviewers, Scientist, ScientistRef, @@ -21,7 +23,8 @@ def sql_for_index(model): # We can't test the DEFAULT_TABLESPACE and DEFAULT_INDEX_TABLESPACE settings # because they're evaluated when the model class is defined. As a consequence, # @override_settings doesn't work, and the tests depend -class TablespacesTests(TestCase): +class TablespacesTests(TransactionTestCase): + available_apps = ['model_options'] def setUp(self): # The unmanaged models need to be removed after the test in order to diff --git a/tests/test_runner/tests.py b/tests/test_runner/tests.py index 708a7c2da7..c20f136b20 100644 --- a/tests/test_runner/tests.py +++ b/tests/test_runner/tests.py @@ -10,6 +10,7 @@ from django import db from django.conf import settings from django.core.exceptions import ImproperlyConfigured from django.core.management import call_command +from django.core.management.base import SystemCheckError from django.test import ( TestCase, TransactionTestCase, skipUnlessDBFeature, testcases, ) @@ -201,8 +202,9 @@ class Ticket17477RegressionTests(AdminScriptTestCase): self.assertNoOutput(err) -class Sqlite3InMemoryTestDbs(TestCase): +class Sqlite3InMemoryTestDbs(TransactionTestCase): multi_db = True + available_apps = ['test_runner'] @unittest.skipUnless(all(db.connections[conn].vendor == 'sqlite' for conn in db.connections), "This is an sqlite-specific issue") |