Fixed #28849 -- Fixed referenced table and column rename on SQLite.

Thanks Ramiro for the input and Tim for the review.

4 files changed, 108 insertions, 18 deletions

diff --git a/tests/backends/models.py b/tests/backends/models.py
index 6277e52525..1fa8d44e63 100644
--- a/tests/backends/models.py
+++ b/tests/backends/models.py
@@ -103,3 +103,11 @@ class ObjectReference(models.Model):
 
 class RawData(models.Model):
     raw_data = models.BinaryField()
+
+
+class Author(models.Model):
+    name = models.CharField(max_length=255, unique=True)
+
+
+class Book(models.Model):
+    author = models.ForeignKey(Author, models.CASCADE, to_field='name')
diff --git a/tests/backends/sqlite/tests.py b/tests/backends/sqlite/tests.py
index 0fbb139278..c82ed1667d 100644
--- a/tests/backends/sqlite/tests.py
+++ b/tests/backends/sqlite/tests.py
@@ -4,10 +4,12 @@ import unittest
 
 from django.db import connection
 from django.db.models import Avg, StdDev, Sum, Variance
+from django.db.models.fields import CharField
 from django.db.utils import NotSupportedError
 from django.test import TestCase, TransactionTestCase, override_settings
+from django.test.utils import isolate_apps
 
-from ..models import Item, Object, Square
+from ..models import Author, Item, Object, Square
 
 
 @unittest.skipUnless(connection.vendor == 'sqlite', 'SQLite tests')
@@ -58,6 +60,44 @@ class Tests(TestCase):
         self.assertEqual(creation._get_test_db_name(), creation.connection.settings_dict['TEST']['NAME'])
 
 
+@unittest.skipUnless(connection.vendor == 'sqlite', 'SQLite tests')
+@isolate_apps('backends')
+class SchemaTests(TransactionTestCase):
+
+    available_apps = ['backends']
+
+    def test_field_rename_inside_atomic_block(self):
+        """
+        NotImplementedError is raised when a model field rename is attempted
+        inside an atomic block.
+        """
+        new_field = CharField(max_length=255, unique=True)
+        new_field.set_attributes_from_name('renamed')
+        msg = (
+            "Renaming the 'backends_author'.'name' column while in a "
+            "transaction is not supported on SQLite because it would break "
+            "referential integrity. Try adding `atomic = False` to the "
+            "Migration class."
+        )
+        with self.assertRaisesMessage(NotSupportedError, msg):
+            with connection.schema_editor(atomic=True) as editor:
+                editor.alter_field(Author, Author._meta.get_field('name'), new_field)
+
+    def test_table_rename_inside_atomic_block(self):
+        """
+        NotImplementedError is raised when a table rename is attempted inside
+        an atomic block.
+        """
+        msg = (
+            "Renaming the 'backends_author' table while in a transaction is "
+            "not supported on SQLite because it would break referential "
+            "integrity. Try adding `atomic = False` to the Migration class."
+        )
+        with self.assertRaisesMessage(NotSupportedError, msg):
+            with connection.schema_editor(atomic=True) as editor:
+                editor.alter_db_table(Author, "backends_author", "renamed_table")
+
+
 @unittest.skipUnless(connection.vendor == 'sqlite', 'Test only for SQLite')
 @override_settings(DEBUG=True)
 class LastExecutedQueryTest(TestCase):
diff --git a/tests/migrations/test_operations.py b/tests/migrations/test_operations.py
index 02f8713650..2e24047d6d 100644
--- a/tests/migrations/test_operations.py
+++ b/tests/migrations/test_operations.py
@@ -28,16 +28,16 @@ class OperationTestBase(MigrationTestBase):
     Common functions to help test operations.
     """
 
-    def apply_operations(self, app_label, project_state, operations):
+    def apply_operations(self, app_label, project_state, operations, atomic=True):
         migration = Migration('name', app_label)
         migration.operations = operations
-        with connection.schema_editor() as editor:
+        with connection.schema_editor(atomic=atomic) as editor:
             return migration.apply(project_state, editor)
 
-    def unapply_operations(self, app_label, project_state, operations):
+    def unapply_operations(self, app_label, project_state, operations, atomic=True):
         migration = Migration('name', app_label)
         migration.operations = operations
-        with connection.schema_editor() as editor:
+        with connection.schema_editor(atomic=atomic) as editor:
             return migration.unapply(project_state, editor)
 
     def make_test_state(self, app_label, operation, **kwargs):
@@ -561,7 +561,8 @@ class OperationTests(OperationTestBase):
             self.assertFKNotExists("test_rnmo_rider", ["pony_id"], ("test_rnmo_horse", "id"))
         # Migrate forwards
         new_state = project_state.clone()
-        new_state = self.apply_operations("test_rnmo", new_state, [operation])
+        atomic_rename = connection.features.supports_atomic_references_rename
+        new_state = self.apply_operations("test_rnmo", new_state, [operation], atomic=atomic_rename)
         # Test new state and database
         self.assertNotIn(("test_rnmo", "pony"), new_state.models)
         self.assertIn(("test_rnmo", "horse"), new_state.models)
@@ -573,7 +574,7 @@ class OperationTests(OperationTestBase):
             self.assertFKNotExists("test_rnmo_rider", ["pony_id"], ("test_rnmo_pony", "id"))
             self.assertFKExists("test_rnmo_rider", ["pony_id"], ("test_rnmo_horse", "id"))
         # Migrate backwards
-        original_state = self.unapply_operations("test_rnmo", project_state, [operation])
+        original_state = self.unapply_operations("test_rnmo", project_state, [operation], atomic=atomic_rename)
         # Test original state and database
         self.assertIn(("test_rnmo", "pony"), original_state.models)
         self.assertNotIn(("test_rnmo", "horse"), original_state.models)
@@ -634,7 +635,8 @@ class OperationTests(OperationTestBase):
         if connection.features.supports_foreign_keys:
             self.assertFKExists("test_rmwsrf_rider", ["friend_id"], ("test_rmwsrf_rider", "id"))
             self.assertFKNotExists("test_rmwsrf_rider", ["friend_id"], ("test_rmwsrf_horserider", "id"))
-        with connection.schema_editor() as editor:
+        atomic_rename = connection.features.supports_atomic_references_rename
+        with connection.schema_editor(atomic=atomic_rename) as editor:
             operation.database_forwards("test_rmwsrf", editor, project_state, new_state)
         self.assertTableNotExists("test_rmwsrf_rider")
         self.assertTableExists("test_rmwsrf_horserider")
@@ -642,7 +644,7 @@ class OperationTests(OperationTestBase):
             self.assertFKNotExists("test_rmwsrf_horserider", ["friend_id"], ("test_rmwsrf_rider", "id"))
             self.assertFKExists("test_rmwsrf_horserider", ["friend_id"], ("test_rmwsrf_horserider", "id"))
         # And test reversal
-        with connection.schema_editor() as editor:
+        with connection.schema_editor(atomic=atomic_rename) as editor:
             operation.database_backwards("test_rmwsrf", editor, new_state, project_state)
         self.assertTableExists("test_rmwsrf_rider")
         self.assertTableNotExists("test_rmwsrf_horserider")
@@ -675,7 +677,7 @@ class OperationTests(OperationTestBase):
             # and the foreign key on rider points to pony, not shetland pony
             self.assertFKExists("test_rmwsc_rider", ["pony_id"], ("test_rmwsc_pony", "id"))
             self.assertFKNotExists("test_rmwsc_rider", ["pony_id"], ("test_rmwsc_shetlandpony", "id"))
-        with connection.schema_editor() as editor:
+        with connection.schema_editor(atomic=connection.features.supports_atomic_references_rename) as editor:
             operation.database_forwards("test_rmwsc", editor, project_state, new_state)
         # Now we have a little horse table, not shetland pony
         self.assertTableNotExists("test_rmwsc_shetlandpony")
@@ -696,7 +698,7 @@ class OperationTests(OperationTestBase):
         ])
         project_state = self.apply_operations(app_label, project_state, operations=[
             migrations.RenameModel("ReflexivePony", "ReflexivePony2"),
-        ])
+        ], atomic=connection.features.supports_atomic_references_rename)
         Pony = project_state.apps.get_model(app_label, "ReflexivePony2")
         pony = Pony.objects.create()
         pony.ponies.add(pony)
@@ -749,7 +751,7 @@ class OperationTests(OperationTestBase):
 
         project_state = self.apply_operations(app_label, project_state, operations=[
             migrations.RenameModel("Rider", "Rider2"),
-        ])
+        ], atomic=connection.features.supports_atomic_references_rename)
         Pony = project_state.apps.get_model(app_label, "Pony")
         Rider = project_state.apps.get_model(app_label, "Rider2")
         pony = Pony.objects.create()
@@ -1397,7 +1399,7 @@ class OperationTests(OperationTestBase):
         project_state = self.apply_operations(app_label, project_state, operations=[
             migrations.RenameField('Rider', 'id', 'id2'),
             migrations.AlterField('Pony', 'id', models.CharField(primary_key=True, max_length=99)),
-        ])
+        ], atomic=connection.features.supports_atomic_references_rename)
 
     def test_rename_field(self):
         """
diff --git a/tests/schema/tests.py b/tests/schema/tests.py
index 66a54b1ce4..10c8b9780b 100644
--- a/tests/schema/tests.py
+++ b/tests/schema/tests.py
@@ -173,7 +173,7 @@ class SchemaTests(TransactionTestCase):
         index_orders = constraints[index]['orders']
         self.assertTrue(all(val == expected for val, expected in zip(index_orders, order)))
 
-    def assertForeignKeyExists(self, model, column, expected_fk_table):
+    def assertForeignKeyExists(self, model, column, expected_fk_table, field='id'):
         """
         Fail if the FK constraint on `model.Meta.db_table`.`column` to
         `expected_fk_table`.id doesn't exist.
@@ -184,7 +184,7 @@ class SchemaTests(TransactionTestCase):
             if details['columns'] == [column] and details['foreign_key']:
                 constraint_fk = details['foreign_key']
                 break
-        self.assertEqual(constraint_fk, (expected_fk_table, 'id'))
+        self.assertEqual(constraint_fk, (expected_fk_table, field))
 
     def assertForeignKeyNotExists(self, model, column, expected_fk_table):
         with self.assertRaises(AssertionError):
@@ -1147,6 +1147,30 @@ class SchemaTests(TransactionTestCase):
         self.assertEqual(columns['display_name'][0], "CharField")
         self.assertNotIn("name", columns)
 
+    @isolate_apps('schema')
+    def test_rename_referenced_field(self):
+        class Author(Model):
+            name = CharField(max_length=255, unique=True)
+
+            class Meta:
+                app_label = 'schema'
+
+        class Book(Model):
+            author = ForeignKey(Author, CASCADE, to_field='name')
+
+            class Meta:
+                app_label = 'schema'
+
+        with connection.schema_editor() as editor:
+            editor.create_model(Author)
+            editor.create_model(Book)
+        new_field = CharField(max_length=255, unique=True)
+        new_field.set_attributes_from_name('renamed')
+        with connection.schema_editor(atomic=connection.features.supports_atomic_references_rename) as editor:
+            editor.alter_field(Author, Author._meta.get_field('name'), new_field)
+        # Ensure the foreign key reference was updated.
+        self.assertForeignKeyExists(Book, 'author_id', 'schema_author', 'renamed')
+
     @skipIfDBFeature('interprets_empty_strings_as_nulls')
     def test_rename_keep_null_status(self):
         """
@@ -1625,25 +1649,41 @@ class SchemaTests(TransactionTestCase):
             ),
         )
 
+    @isolate_apps('schema')
     def test_db_table(self):
         """
         Tests renaming of the table
         """
-        # Create the table
+        class Author(Model):
+            name = CharField(max_length=255)
+
+            class Meta:
+                app_label = 'schema'
+
+        class Book(Model):
+            author = ForeignKey(Author, CASCADE)
+
+            class Meta:
+                app_label = 'schema'
+
+        # Create the table and one referring it.
         with connection.schema_editor() as editor:
             editor.create_model(Author)
+            editor.create_model(Book)
         # Ensure the table is there to begin with
         columns = self.column_classes(Author)
         self.assertEqual(columns['name'][0], "CharField")
         # Alter the table
-        with connection.schema_editor() as editor:
+        with connection.schema_editor(atomic=connection.features.supports_atomic_references_rename) as editor:
             editor.alter_db_table(Author, "schema_author", "schema_otherauthor")
         # Ensure the table is there afterwards
         Author._meta.db_table = "schema_otherauthor"
         columns = self.column_classes(Author)
         self.assertEqual(columns['name'][0], "CharField")
+        # Ensure the foreign key reference was updated
+        self.assertForeignKeyExists(Book, "author_id", "schema_otherauthor")
         # Alter the table again
-        with connection.schema_editor() as editor:
+        with connection.schema_editor(atomic=connection.features.supports_atomic_references_rename) as editor:
             editor.alter_db_table(Author, "schema_otherauthor", "schema_author")
         # Ensure the table is still there
         Author._meta.db_table = "schema_author"