diff options
| -rw-r--r-- | django/db/backends/mysql/schema.py | 8 | ||||
| -rw-r--r-- | django/db/backends/postgresql_psycopg2/schema.py | 9 | ||||
| -rw-r--r-- | django/db/backends/schema.py | 66 | ||||
| -rw-r--r-- | docs/releases/1.7.8.txt | 3 | ||||
| -rw-r--r-- | tests/migrations/test_operations.py | 13 | ||||
| -rw-r--r-- | tests/schema/tests.py | 20 |
6 files changed, 90 insertions, 29 deletions
diff --git a/django/db/backends/mysql/schema.py b/django/db/backends/mysql/schema.py index bce1a158ba..37c130e908 100644 --- a/django/db/backends/mysql/schema.py +++ b/django/db/backends/mysql/schema.py @@ -48,3 +48,11 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor): 'table': self.quote_name(model._meta.db_table), 'column': self.quote_name(field.column), }, [effective_default]) + + def _alter_column_type_sql(self, table, old_field, new_field, new_type): + # Keep null property of old field, if it has changed, it will be handled separately + if old_field.null: + new_type += " NULL" + else: + new_type += " NOT NULL" + return super(DatabaseSchemaEditor, self)._alter_column_type_sql(table, old_field, new_field, new_type) diff --git a/django/db/backends/postgresql_psycopg2/schema.py b/django/db/backends/postgresql_psycopg2/schema.py index 7b479c28b0..6caf38df81 100644 --- a/django/db/backends/postgresql_psycopg2/schema.py +++ b/django/db/backends/postgresql_psycopg2/schema.py @@ -34,11 +34,12 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor): model, [field], suffix='_like', sql=self.sql_create_text_index)) return output - def _alter_column_type_sql(self, table, column, type): + def _alter_column_type_sql(self, table, old_field, new_field, new_type): """ Makes ALTER TYPE with SERIAL make sense. """ - if type.lower() == "serial": + if new_type.lower() == "serial": + column = new_field.column sequence_name = "%s_%s_seq" % (table, column) return ( ( @@ -82,4 +83,6 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor): ], ) else: - return super(DatabaseSchemaEditor, self)._alter_column_type_sql(table, column, type) + return super(DatabaseSchemaEditor, self)._alter_column_type_sql( + table, old_field, new_field, new_type + ) diff --git a/django/db/backends/schema.py b/django/db/backends/schema.py index 4a5637c8da..6febdb134d 100644 --- a/django/db/backends/schema.py +++ b/django/db/backends/schema.py @@ -13,6 +13,14 @@ from django.utils import six logger = getLogger('django.db.backends.schema') +def _related_objects(old_field, new_field): + # Returns (old_relation, new_relation) tuples. + return zip( + old_field.model._meta.get_all_related_objects(), + new_field.model._meta.get_all_related_objects() + ) + + class BaseDatabaseSchemaEditor(object): """ This class (and its subclasses) are responsible for emitting schema-changing @@ -438,12 +446,17 @@ class BaseDatabaseSchemaEditor(object): old_type = old_db_params['type'] new_db_params = new_field.db_parameters(connection=self.connection) new_type = new_db_params['type'] - if (old_type is None and old_field.rel is None) or (new_type is None and new_field.rel is None): - raise ValueError("Cannot alter field %s into %s - they do not properly define db_type (are you using PostGIS 1.5 or badly-written custom fields?)" % ( - old_field, - new_field, - )) - elif old_type is None and new_type is None and (old_field.rel.through and new_field.rel.through and old_field.rel.through._meta.auto_created and new_field.rel.through._meta.auto_created): + if ((old_type is None and old_field.rel is None) or + (new_type is None and new_field.rel is None)): + raise ValueError( + "Cannot alter field %s into %s - they do not properly define " + "db_type (are you using PostGIS 1.5 or badly-written custom " + "fields?)" % (old_field, new_field) + ) + elif old_type is None and new_type is None and ( + old_field.rel.through and new_field.rel.through and + old_field.rel.through._meta.auto_created and + new_field.rel.through._meta.auto_created): return self._alter_many_to_many(model, old_field, new_field, strict) elif old_type is None and new_type is None and (old_field.rel.through and new_field.rel.through and not old_field.rel.through._meta.auto_created and not new_field.rel.through._meta.auto_created): # Both sides have through models; this is a no-op. @@ -487,10 +500,12 @@ class BaseDatabaseSchemaEditor(object): # Drop incoming FK constraints if we're a primary key and things are going # to change. if old_field.primary_key and new_field.primary_key and old_type != new_type: - for rel in new_field.model._meta.get_all_related_objects(): - rel_fk_names = self._constraint_names(rel.model, [rel.field.column], foreign_key=True) + for _old_rel, new_rel in _related_objects(old_field, new_field): + rel_fk_names = self._constraint_names( + new_rel.model, [new_rel.field.column], foreign_key=True + ) for fk_name in rel_fk_names: - self.execute(self._delete_constraint_sql(self.sql_delete_fk, rel.model, fk_name)) + self.execute(self._delete_constraint_sql(self.sql_delete_fk, new_rel.model, fk_name)) # Removed an index? (no strict check, as multiple indexes are possible) if (old_field.db_index and not new_field.db_index and not old_field.unique and not @@ -524,7 +539,9 @@ class BaseDatabaseSchemaEditor(object): post_actions = [] # Type change? if old_type != new_type: - fragment, other_actions = self._alter_column_type_sql(model._meta.db_table, new_field.column, new_type) + fragment, other_actions = self._alter_column_type_sql( + model._meta.db_table, old_field, new_field, new_type + ) actions.append(fragment) post_actions.extend(other_actions) # When changing a column NULL constraint to NOT NULL with a given @@ -637,7 +654,7 @@ class BaseDatabaseSchemaEditor(object): # referring to us. rels_to_update = [] if old_field.primary_key and new_field.primary_key and old_type != new_type: - rels_to_update.extend(new_field.model._meta.get_all_related_objects()) + rels_to_update.extend(_related_objects(old_field, new_field)) # Changed to become primary key? # Note that we don't detect unsetting of a PK, as we assume another field # will always come along and replace it. @@ -660,20 +677,23 @@ class BaseDatabaseSchemaEditor(object): } ) # Update all referencing columns - rels_to_update.extend(new_field.model._meta.get_all_related_objects()) + rels_to_update.extend(_related_objects(old_field, new_field)) # Handle our type alters on the other end of rels from the PK stuff above - for rel in rels_to_update: - rel_db_params = rel.field.db_parameters(connection=self.connection) + for old_rel, new_rel in rels_to_update: + rel_db_params = new_rel.field.db_parameters(connection=self.connection) rel_type = rel_db_params['type'] + fragment, other_actions = self._alter_column_type_sql( + new_rel.model._meta.db_table, old_rel.field, new_rel.field, rel_type + ) self.execute( self.sql_alter_column % { - "table": self.quote_name(rel.model._meta.db_table), - "changes": self.sql_alter_column_type % { - "column": self.quote_name(rel.field.column), - "type": rel_type, - } - } + "table": self.quote_name(new_rel.model._meta.db_table), + "changes": fragment[0], + }, + fragment[1], ) + for sql, params in other_actions: + self.execute(sql, params) # Does it have a foreign key? if (new_field.rel and (fks_dropped or not old_field.rel or not old_field.db_constraint) and @@ -707,7 +727,7 @@ class BaseDatabaseSchemaEditor(object): if self.connection.features.connection_persists_old_columns: self.connection.close() - def _alter_column_type_sql(self, table, column, type): + def _alter_column_type_sql(self, table, old_field, new_field, new_type): """ Hook to specialize column type alteration for different backends, for cases when a creation type is different to an alteration type @@ -720,8 +740,8 @@ class BaseDatabaseSchemaEditor(object): return ( ( self.sql_alter_column_type % { - "column": self.quote_name(column), - "type": type, + "column": self.quote_name(new_field.column), + "type": new_type, }, [], ), diff --git a/docs/releases/1.7.8.txt b/docs/releases/1.7.8.txt index 912171f8da..dc281b6b62 100644 --- a/docs/releases/1.7.8.txt +++ b/docs/releases/1.7.8.txt @@ -10,3 +10,6 @@ Django 1.7.8 fixes: (:ticket:`24637`). * A database table name quoting regression in 1.7.2 (:ticket:`24605`). + +* Prevented the loss of ``null``/``not null`` column properties during field + alteration of MySQL databases (:ticket:`24595`). diff --git a/tests/migrations/test_operations.py b/tests/migrations/test_operations.py index 3cc225632a..180a3c852b 100644 --- a/tests/migrations/test_operations.py +++ b/tests/migrations/test_operations.py @@ -1032,9 +1032,18 @@ class OperationTests(OperationTestBase): def assertIdTypeEqualsFkType(): with connection.cursor() as cursor: - id_type = [c.type_code for c in connection.introspection.get_table_description(cursor, "test_alflpkfk_pony") if c.name == "id"][0] - fk_type = [c.type_code for c in connection.introspection.get_table_description(cursor, "test_alflpkfk_rider") if c.name == "pony_id"][0] + id_type, id_null = [ + (c.type_code, c.null_ok) + for c in connection.introspection.get_table_description(cursor, "test_alflpkfk_pony") + if c.name == "id" + ][0] + fk_type, fk_null = [ + (c.type_code, c.null_ok) + for c in connection.introspection.get_table_description(cursor, "test_alflpkfk_rider") + if c.name == "pony_id" + ][0] self.assertEqual(id_type, fk_type) + self.assertEqual(id_null, fk_null) assertIdTypeEqualsFkType() # Test the database alteration diff --git a/tests/schema/tests.py b/tests/schema/tests.py index cc95d5539e..a0fa8958d7 100644 --- a/tests/schema/tests.py +++ b/tests/schema/tests.py @@ -29,7 +29,7 @@ class SchemaTests(TransactionTestCase): Author, AuthorWithM2M, Book, BookWithLongName, BookWithSlug, BookWithM2M, Tag, TagIndexed, TagM2MTest, TagUniqueRename, UniqueTest, Thing, TagThrough, BookWithM2MThrough, AuthorWithEvenLongerName, - BookWeak, BookWithO2O, BookWithoutFK, + BookWeak, BookWithO2O, BookWithoutFK, Note, ] # Utility functions @@ -427,6 +427,8 @@ class SchemaTests(TransactionTestCase): def test_alter_text_field(self): # Regression for "BLOB/TEXT column 'info' can't have a default value") # on MySQL. + with connection.schema_editor() as editor: + editor.create_model(Note) new_field = TextField(blank=True) new_field.set_attributes_from_name("info") with connection.schema_editor() as editor: @@ -437,6 +439,22 @@ class SchemaTests(TransactionTestCase): strict=True, ) + def test_alter_field_keep_null_status(self): + """ + Changing a field type shouldn't affect the not null status. + """ + with connection.schema_editor() as editor: + editor.create_model(Note) + with self.assertRaises(IntegrityError): + Note.objects.create(info=None) + old_field = Note._meta.get_field("info") + new_field = CharField(max_length=50) + new_field.set_attributes_from_name("info") + with connection.schema_editor() as editor: + editor.alter_field(Note, old_field, new_field, strict=True) + with self.assertRaises(IntegrityError): + Note.objects.create(info=None) + def test_alter_null_to_not_null(self): """ #23609 - Tests handling of default values when altering from NULL to NOT NULL. |