Opened 4 years ago
Closed 4 years ago
#33197 closed Cleanup/optimization (fixed)
Renaming field and providing prior field name to db_column should be an SQL noop
| Reported by: | Jacob Walls | Owned by: | Simon Charette |
|---|---|---|---|
| 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 (7)
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'),
),
]
comment:3 by , 4 years ago
You'll want to adjust AlterField.name accordingly if you swap the order of operations; change name='core_renamed' to name='core'.
operations = [ migrations.AlterField( model_name='apple', name='core', field=models.BooleanField(db_column='core'), ), migrations.RenameField( model_name='apple', old_name='core', new_name='core_renamed', ), ]
comment:4 by , 4 years ago
| Has patch: | set |
|---|
comment:5 by , 4 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:6 by , 4 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
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.