[1.8.x] Fixed #24595 -- Prevented loss of null info in MySQL field alteration

Thanks Simon Percivall for the report, and Simon Charette and Tim
Graham for the reviews.
Backport of 02260ea3f6 from master.
This commit is contained in:
Claude Paroz 2015-04-11 16:10:31 +02:00
parent d0b18542f7
commit bbfcd9618b
7 changed files with 80 additions and 26 deletions

View File

@ -9,9 +9,13 @@ from django.utils.log import getLogger
logger = getLogger('django.db.backends.schema')
def _related_non_m2m_objects(opts):
# filters out m2m objects from reverse relations.
return (obj for obj in opts.related_objects if not obj.field.many_to_many)
def _related_non_m2m_objects(old_field, new_field):
# Filters out m2m objects from reverse relations.
# Returns (old_relation, new_relation) tuples.
return zip(
(obj for obj in old_field.model._meta.related_objects if not obj.field.many_to_many),
(obj for obj in new_field.model._meta.related_objects if not obj.field.many_to_many)
)
class BaseDatabaseSchemaEditor(object):
@ -456,7 +460,8 @@ 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):
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 "
@ -517,10 +522,12 @@ class BaseDatabaseSchemaEditor(object):
if old_field.primary_key and new_field.primary_key and old_type != new_type:
# '_meta.related_field' also contains M2M reverse fields, these
# will be filtered out
for rel in _related_non_m2m_objects(new_field.model._meta):
rel_fk_names = self._constraint_names(rel.related_model, [rel.field.column], foreign_key=True)
for _old_rel, new_rel in _related_non_m2m_objects(old_field, new_field):
rel_fk_names = self._constraint_names(
new_rel.related_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.related_model, fk_name))
self.execute(self._delete_constraint_sql(self.sql_delete_fk, new_rel.related_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
@ -554,7 +561,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
@ -671,7 +680,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(_related_non_m2m_objects(new_field.model._meta))
rels_to_update.extend(_related_non_m2m_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.
@ -694,20 +703,23 @@ class BaseDatabaseSchemaEditor(object):
}
)
# Update all referencing columns
rels_to_update.extend(_related_non_m2m_objects(new_field.model._meta))
rels_to_update.extend(_related_non_m2m_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.related_model._meta.db_table, old_rel.field, new_rel.field, rel_type
)
self.execute(
self.sql_alter_column % {
"table": self.quote_name(rel.related_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.related_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
@ -742,7 +754,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
@ -755,8 +767,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,
},
[],
),

View File

@ -61,3 +61,11 @@ class DatabaseSchemaEditor(BaseDatabaseSchemaEditor):
# index creation for FKs (index automatically created by MySQL)
field.db_index = False
return super(DatabaseSchemaEditor, self)._model_indexes_sql(model)
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)

View File

@ -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
)

View File

@ -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`).

View File

@ -55,3 +55,6 @@ Bugfixes
``qs.annotate(foo=F('field')).values('pk').order_by('foo'))`` (:ticket:`24615`).
* Fixed a database table name quoting regression (:ticket:`24605`).
* Prevented the loss of ``null``/``not null`` column properties during field
alteration of MySQL databases (:ticket:`24595`).

View File

@ -1101,9 +1101,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

View File

@ -426,6 +426,22 @@ class SchemaTests(TransactionTestCase):
with connection.schema_editor() as editor:
editor.alter_field(Note, old_field, new_field, 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.