Incorrect migration generated if field has been renamed when it has verbose_name set

[Peter Inglesby]

If you start with following model:

class Thing(models.Model):
    name = models.CharField(max_length=100, default="xxx", verbose_name="name")

and change it to:

class Thing(models.Model):
    label = models.CharField(max_length=100, default="xxx", verbose_name="name")

Django asks:

Did you rename thing.name to thing.label (a CharField)? [y/N] y

and if you say yes, it creates a migration with:

    operations = [
        migrations.RenameField(
            model_name='thing',
            old_name='name',
            new_name='label',
        ),
    ]

But if you change it to:

class thing(models.model):
    label = models.CharField(max_length=100, default="xxx", verbose_name="label")

if creates a migration with:

    operations = [
        migrations.RemoveField(
            model_name='thing',
            name='name',
        ),
        migrations.AddField(
            model_name='thing',
            name='label',
            field=models.CharField(default='xxx', max_length=100, verbose_name='label'),
        ),
    ]

If you were to run this migration without thinking, you would risk losing data.

[Carlton Gibson]

Hi Peter. Nice example. Yes, that does seem curious. I can confirm the behavior. Didn't look into why it happens as yet.

[Simon Charette]

Detection of field renames happens in the auto-detector's generate_renamed_fields method

​https://github.com/django/django/blob/4a412c2e659f9295973434c65b785b03acee7251/django/db/migrations/autodetector.py#L837-L841

I guess it could be adjusted to ignore verbose_name when comparing fields like we do with db_column by storing old_field_verbose_name = old_field.verbose_name and setting verbose_name=old_field_verbose_name in the comparison against field_dec[2].

[Simon Charette]

It feels like it's more of an optimization than a bug though as this behaviour is due to the initial design decision that the migration framework should not hold an allow list of attributes that affect database state or not. The same problem is present when altering validators for example.

[Hasan Ramezani]

Thanks, Simon for the proposed solution to fix the problem.
I just thought maybe we can remove verbose_name from old_field_dec and field_dec and then compare them.
I don't know this is the right approach or not.

​PR

[Mariusz Felisiak]

Replying to Simon Charette:

It feels like it's more of an optimization than a bug though as this behaviour is due to the initial design decision that the migration framework should not hold an allow list of attributes that affect database state or not. The same problem is present when altering validators for example.

Yes it's not a bug, maybe a cleanup, but I'm not sure about changing the current behavior. There are multiple attributes that don't affect database state, see ​BaseDatabaseSchemaEditor._field_should_be_altered(), but we cannot guarantee this for 3rd-party backends. I would close this as wontfix and keep the current flow, so if you want to rename a field and change its non-database attributes you need to do this in two steps:

• rename field (RenameField),
• change attributes (AlterField, which is a noop operation for such attributes, see #25253 and #31826).

[Carlton Gibson]

Hi All — thanks for the input.

Can I ask, can we warn in these kind of cases somehow?

So, a user should do it in two steps, and users should review generated migrations to make sure the operations are what's required, but I think this probably counts as a loaded foot-gun.

Perhaps we should re-open #31700 which suggested adding additional output for destructive operations? (Mailing list summary there was "Yes, maybe something at the makemigrations stage...) — What do we think? (Thanks!)

[Carlton Gibson]

OK, given Simon ans Mariusz' comments lets take this as a duplicate of #31700 and aim to provide a better warning for this kind of case.