Opened 4 years ago
Last modified 4 years ago
#33197 closed Cleanup/optimization
Renaming field and providing prior field name to db_column should be an SQL noop — at Version 2
| Reported by: | Jacob Walls | Owned by: | nobody | 
|---|---|---|---|
| Component: | Migrations | Version: | dev | 
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no | 
| Needs tests: | no | Patch needs improvement: | no | 
| Easy pickings: | no | UI/UX: | no | 
Description (last modified by )
Renaming a field and setting the prior implicit field name as the db_column to avoid db operations creates a migration emitting unnecessary SQL. Similar to #31826, which handled a very similar scenario but where there is no field rename, I would expect a SQL noop. I tested with SQLite and MySQL 5.7.31. 
class Apple(models.Model):
    core = models.BooleanField()
class Apple(models.Model):
    core_renamed = models.BooleanField(db_column='core')
Was apple.core renamed to apple.core_renamed (a BooleanField)? [y/N] y
Migrations for 'renamez':
  renamez/migrations/0002_rename_core_apple_core_renamed_and_more.py
    - Rename field core on apple to core_renamed
    - Alter field core_renamed on apple
python manage.py sqlmigrate renamez 0002 showing unnecessary SQL:
BEGIN; -- -- Rename field core on apple to core_renamed -- ALTER TABLE "renamez_apple" RENAME COLUMN "core" TO "core_renamed"; -- -- Alter field core_renamed on apple -- ALTER TABLE "renamez_apple" RENAME COLUMN "core_renamed" TO "core"; COMMIT;
Without renaming the field, follow the same flow and get an AlterField migration without SQL, which is what #31826 intended:
BEGIN; -- -- Alter field core on apple -- COMMIT;
Change History (2)
comment:1 by , 4 years ago
| Triage Stage: | Unreviewed → Accepted | 
|---|
comment:2 by , 4 years ago
| Description: | modified (diff) | 
|---|---|
| Summary: | Providing prior implicit field name to db_column should be a noop → Renaming field and providing prior field name to db_column should be an SQL noop | 
Thanks for having a look. I see now the scope of #31826 was just for flows where the field is not renamed. So that makes this ticket a request to extend this to field renames, which looks like was discussed as 3 and 4 here.
I assume the issue goes away if you swap the order of operations in your migration?
If I switch the order to have AlterField followed by RenameField, FieldDoesNotExist is raised when migrating. These are the operations:
    operations = [
        migrations.RenameField(
            model_name='apple',
            old_name='core',
            new_name='core_renamed',
        ),
        migrations.AlterField(
            model_name='apple',
            name='core_renamed',
            field=models.BooleanField(db_column='core'),
        ),
    ]
I think this is due to how the auto-detector generates field renames before field alterations. I assume the issue goes away if you swap the order of
operationsin your migration?As this test exemplifies the
RenameFieldoperation happens before theAlterFieldoperation does so we'd need to adjustgenerate_renamed_fieldsto add anAlterFieldand preventsgenerate_altered_fieldsfor doing so when this happens.