[1.7.x] Fixed #23609 -- Fixed IntegrityError that prevented altering a NULL column into a NOT NULL one due to existing rows

Thanks to Simon Charette, Loic Bistuer and Tim Graham for the review.

Backport of f633ba778d from master

2 files changed, 109 insertions, 1 deletions

diff --git a/tests/migrations/test_autodetector.py b/tests/migrations/test_autodetector.py
index c58b41de30..56596309be 100644
--- a/tests/migrations/test_autodetector.py
+++ b/tests/migrations/test_autodetector.py
@@ -26,6 +26,7 @@ class AutodetectorTests(TestCase):
 
     author_empty = ModelState("testapp", "Author", [("id", models.AutoField(primary_key=True))])
     author_name = ModelState("testapp", "Author", [("id", models.AutoField(primary_key=True)), ("name", models.CharField(max_length=200))])
+    author_name_null = ModelState("testapp", "Author", [("id", models.AutoField(primary_key=True)), ("name", models.CharField(max_length=200, null=True))])
     author_name_longer = ModelState("testapp", "Author", [("id", models.AutoField(primary_key=True)), ("name", models.CharField(max_length=400))])
     author_name_renamed = ModelState("testapp", "Author", [("id", models.AutoField(primary_key=True)), ("names", models.CharField(max_length=200))])
     author_name_default = ModelState("testapp", "Author", [("id", models.AutoField(primary_key=True)), ("name", models.CharField(max_length=200, default='Ada Lovelace'))])
@@ -275,6 +276,80 @@ class AutodetectorTests(TestCase):
         action = migration.operations[0]
         self.assertEqual(action.__class__.__name__, "AlterField")
         self.assertEqual(action.name, "name")
+        self.assertTrue(action.preserve_default)
+
+    def test_alter_field_to_not_null_with_default(self):
+        "#23609 - Tests autodetection of nullable to non-nullable alterations"
+        class CustomQuestioner(MigrationQuestioner):
+            def ask_not_null_alteration(self, field_name, model_name):
+                raise Exception("Should not have prompted for not null addition")
+
+        # Make state
+        before = self.make_project_state([self.author_name_null])
+        after = self.make_project_state([self.author_name_default])
+        autodetector = MigrationAutodetector(before, after, CustomQuestioner())
+        changes = autodetector._detect_changes()
+        # Right number of migrations?
+        self.assertEqual(len(changes['testapp']), 1)
+        # Right number of actions?
+        migration = changes['testapp'][0]
+        self.assertEqual(len(migration.operations), 1)
+        # Right action?
+        action = migration.operations[0]
+        self.assertEqual(action.__class__.__name__, "AlterField")
+        self.assertEqual(action.name, "name")
+        self.assertTrue(action.preserve_default)
+        self.assertEqual(action.field.default, 'Ada Lovelace')
+
+    def test_alter_field_to_not_null_without_default(self):
+        "#23609 - Tests autodetection of nullable to non-nullable alterations"
+        class CustomQuestioner(MigrationQuestioner):
+            def ask_not_null_alteration(self, field_name, model_name):
+                # Ignore for now, and let me handle existing rows with NULL
+                # myself (e.g. adding a RunPython or RunSQL operation in the new
+                # migration file before the AlterField operation)
+                return models.NOT_PROVIDED
+
+        # Make state
+        before = self.make_project_state([self.author_name_null])
+        after = self.make_project_state([self.author_name])
+        autodetector = MigrationAutodetector(before, after, CustomQuestioner())
+        changes = autodetector._detect_changes()
+        # Right number of migrations?
+        self.assertEqual(len(changes['testapp']), 1)
+        # Right number of actions?
+        migration = changes['testapp'][0]
+        self.assertEqual(len(migration.operations), 1)
+        # Right action?
+        action = migration.operations[0]
+        self.assertEqual(action.__class__.__name__, "AlterField")
+        self.assertEqual(action.name, "name")
+        self.assertTrue(action.preserve_default)
+        self.assertIs(action.field.default, models.NOT_PROVIDED)
+
+    def test_alter_field_to_not_null_oneoff_default(self):
+        "#23609 - Tests autodetection of nullable to non-nullable alterations"
+        class CustomQuestioner(MigrationQuestioner):
+            def ask_not_null_alteration(self, field_name, model_name):
+                # Provide a one-off default now (will be set on all existing rows)
+                return 'Some Name'
+
+        # Make state
+        before = self.make_project_state([self.author_name_null])
+        after = self.make_project_state([self.author_name])
+        autodetector = MigrationAutodetector(before, after, CustomQuestioner())
+        changes = autodetector._detect_changes()
+        # Right number of migrations?
+        self.assertEqual(len(changes['testapp']), 1)
+        # Right number of actions?
+        migration = changes['testapp'][0]
+        self.assertEqual(len(migration.operations), 1)
+        # Right action?
+        action = migration.operations[0]
+        self.assertEqual(action.__class__.__name__, "AlterField")
+        self.assertEqual(action.name, "name")
+        self.assertFalse(action.preserve_default)
+        self.assertEqual(action.field.default, "Some Name")
 
     def test_rename_field(self):
         "Tests autodetection of renamed fields"
diff --git a/tests/schema/tests.py b/tests/schema/tests.py
index 02207ff6e9..5b6a23bee0 100644
--- a/tests/schema/tests.py
+++ b/tests/schema/tests.py
@@ -3,7 +3,8 @@ import unittest
 
 from django.test import TransactionTestCase
 from django.db import connection, DatabaseError, IntegrityError, OperationalError
-from django.db.models.fields import IntegerField, TextField, CharField, SlugField, BooleanField, BinaryField
+from django.db.models.fields import (BinaryField, BooleanField, CharField, IntegerField,
+    PositiveIntegerField, SlugField, TextField)
 from django.db.models.fields.related import ManyToManyField, ForeignKey
 from django.db.transaction import atomic
 from .models import (Author, AuthorWithM2M, Book, BookWithLongName,
@@ -422,6 +423,38 @@ class SchemaTests(TransactionTestCase):
         self.assertEqual(columns['name'][0], "TextField")
         self.assertEqual(bool(columns['name'][1][6]), False)
 
+    def test_alter_null_to_not_null(self):
+        """
+        #23609 - Tests handling of default values when altering from NULL to NOT NULL.
+        """
+        # Create the table
+        with connection.schema_editor() as editor:
+            editor.create_model(Author)
+        # Ensure the field is right to begin with
+        columns = self.column_classes(Author)
+        self.assertTrue(columns['height'][1][6])
+        # Create some test data
+        Author.objects.create(name='Not null author', height=12)
+        Author.objects.create(name='Null author')
+        # Verify null value
+        self.assertEqual(Author.objects.get(name='Not null author').height, 12)
+        self.assertIsNone(Author.objects.get(name='Null author').height)
+        # Alter the height field to NOT NULL with default
+        new_field = PositiveIntegerField(default=42)
+        new_field.set_attributes_from_name("height")
+        with connection.schema_editor() as editor:
+            editor.alter_field(
+                Author,
+                Author._meta.get_field_by_name("height")[0],
+                new_field
+            )
+        # Ensure the field is right afterwards
+        columns = self.column_classes(Author)
+        self.assertFalse(columns['height'][1][6])
+        # Verify default value
+        self.assertEqual(Author.objects.get(name='Not null author').height, 12)
+        self.assertEqual(Author.objects.get(name='Null author').height, 42)
+
     @unittest.skipUnless(connection.features.supports_foreign_keys, "No FK support")
     def test_alter_fk(self):
         """