Opened 10 years ago
Last modified 4 months ago
#23577 assigned Bug
Rename operations should rename indexes, constraints, sequences and triggers named after their former value
Reported by: | Chris Woytowitz | Owned by: | Victor Rocha |
---|---|---|---|
Component: | Migrations | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | bugs@…, aksheshdoshi@…, emorley@…, Ryan Hiebert, Vadym Moshynskyi, Thomas Riccardi, Sardorbek Imomaliev, Friedrich Delgado, Daniel Hahler, Bhuvnesh | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
This one's a bit of an edge case, but I ran into it trying to refactor some models on my moderately large work project.
Let's say I have a Django 1.7 app called sample
:
- Create a model
Bar
. - Create a model
Foo
withbar = models.ForeignKey(Bar, blank=True, null=True)
makemigrations
(migration0001
)- Rename
Foo
toOldFoo
. makemigrations
, say yes to the rename prompt (migration0002
)- Create new model
Foo
, which also hasbar = models.ForeignKey(Bar, blank=True, null=True)
makemigrations
(migration0003
)migrate
When migrate
hits 0003
, it throws this error:
django.db.utils.OperationalError: index sample_foo_4350f7d0 already exists
You may notice that sqlmigrate sample 0001
and sqlmigrate sample 0003
have the same line in both of them:
CREATE INDEX sample_foo_4350f7d0 ON "sample_foo" ("bar_id");
In my production case, I actually had no problems on servers, because South had created the index with a different name. When I renamed the models and added a field, the new index did not collide with the old one. However, our test suite started failing, because it would run the migrations from the ground up, exposing the above bug.
I haven't decided on a workaround yet, but I thought I'd bring this to your attention. I might have to inject a migration that renames the index created in 0001
, or something to that effect.
The attached test case has a project dj17test
in the state after having performed all of the above steps.
Attachments (1)
Change History (41)
by , 10 years ago
Attachment: | dj17test.zip added |
---|
comment:1 by , 10 years ago
comment:2 by , 10 years ago
Triage Stage: | Unreviewed → Accepted |
---|
Haven't reproduced but the issue seems legit from the report.
I guess the RenameModel
operation should also rename indexes generated from the original model name.
comment:3 by , 10 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
follow-up: 8 comment:4 by , 10 years ago
I've found an associated case where a field with db_index=True
, is renamed, and another db_indexed field created with the original's name.
Just discussed this with @andrewgodwin, I've agreed to write a test case for the field rename case, and then post about a solution on django-dev, with his preference being to make index names randomised.
comment:5 by , 10 years ago
I've added a schema test case: https://github.com/tomviner/django/compare/ticket_23577_with_tests
This should currently fail on all backends except sqlite (which doesn't rename fields - it drop/creates the whole table). When this bug is fixed, it should pass for all backends, for that reason I haven't added any unittest.skip decorator.
Edit: link to tests updated.
comment:6 by , 10 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
After a discussion on django-dev it makes sense to leave the resolution of this issue to Marc Tamlyn's Advanced indexes DEP, which he says will involve user specified/reproducible index names, as well as index renaming.
comment:7 by , 10 years ago
Cc: | added |
---|
follow-up: 11 comment:8 by , 10 years ago
Replying to tomviner:
I've found an associated case where a field with
db_index=True
, is renamed, and another db_indexed field created with the original's name.
FWIW I have worked around this in my migrations by doing two AlterFields, dropping & re-creating the index by doing db_index=False then db_index=True.
comment:9 by , 9 years ago
This also happens when renaming a ManyToManyField and then adding a new ManyToManyField with the same name. I've opened a ticket then @timgraham pointed out that it's duplicate of this one
https://code.djangoproject.com/ticket/25752
comment:10 by , 9 years ago
I tried to migrate to a new model and bumped into this one. To fix, I made sure that I renamed my existing model to "old" first (e.g. Model->ModelOld), create the new model with the previous name (e.g. Model), migrate the data and then remove the old model. My previous strategy of creating the new table migrating the data and then renaming it broke due to the index issues.
comment:11 by , 9 years ago
Replying to ris:
Replying to tomviner:
I've found an associated case where a field with
db_index=True
, is renamed, and another db_indexed field created with the original's name.
FWIW I have worked around this in my migrations by doing two AlterFields, dropping & re-creating the index by doing db_index=False then db_index=True.
Confirmed. This worked for me also. I can use manage.py sqlmigrate <app> <migration>
to see which indexes are giving me conflicts (should be the same as the error you get when you try to run migrate) and then used the db_index=False/True
trick with AlterField
.
comment:12 by , 9 years ago
Cc: | added |
---|
comment:13 by , 9 years ago
Summary: | CREATE INDEX-related OperationalError on migrating renamed models with colliding field names → RenameModel should rename indexes and constraints named after its old table name |
---|---|
Version: | 1.7 → master |
#24528 was a duplicate.
comment:14 by , 9 years ago
Summary: | RenameModel should rename indexes and constraints named after its old table name → RenameModel should rename indexes, constraints, sequences and triggers named after its old table name |
---|
#24715 was duplicate with a patch for the Oracle case where the sequence and triggers created for an AutoField
are not renamed.
comment:15 by , 9 years ago
Summary: | RenameModel should rename indexes, constraints, sequences and triggers named after its old table name → Rename operations should rename indexes, constraints, sequences and triggers named after their former value |
---|
This should also be the case for AlterField
operations on both sides (e.g. foreign key constraint names are generated from their referenced table and column names).
comment:16 by , 8 years ago
Cc: | added |
---|
comment:17 by , 7 years ago
Until this is resolved, one can work-around this issue by doing something like:
migrations.RunSQL( 'ALTER INDEX myapp_mymodel_myfield_othermodel_id_0f4cfc54 rename TO myapp_mymodel_myfield_othermodel_legacy_id_0f4cfc54', 'ALTER INDEX myapp_mymodel_myfield_othermodel_legacy_id_0f4cfc54 rename TO myapp_mymodel_myfield_othermodel_id_0f4cfc54', )
This sort of approach should work well for both FKs and M2Ms (example below), but it may make your migrations DB Backend specific =/
comment:19 by , 7 years ago
Cc: | added |
---|
comment:20 by , 7 years ago
I just ran into this issue with RenameField
on ManyToMany
relations. The (automatically generated) through table changes name, but the id_seq for it doesn't.
comment:21 by , 6 years ago
Cc: | added |
---|
comment:22 by , 6 years ago
Cc: | added |
---|
comment:23 by , 6 years ago
Some analysis (with postgresql backend):
- when creating a new field with index (such as a ForeignKey), django creates the column and the index
- when deleting a field, django only deletes the column: it lets postgresql delete all related index automatically
- when renaming a field, django only renames the column: it lets postgresql update all index references (and probably more), *but does not* rename the index
It seems django doesn't really track the index name, and it works great except for renames
=> when a new index has to be created for the old field name, the index name conflicts: this is this issue.
This reliance on field reference automatic update by the db seems to create a similar issue with index_together/unique_together:
field rename + index_together/unique_together alternation to update the field name results in no SQL except the RENAME COLUMN: the index/unique names are *not* updated.
The workaround from comment 17 (ticket:23577#comment:17) with a manual index rename may create issues when this ticket is fixed:
at that point django will probably assume the index name is the one it generates if it had to create the index, and the comment uses a different one (the hash_suffix_part is different)
=> for more rubustness/future compatibility I suggest to use the "proper" index name: manually create migrations to delete then re-create the field, and use ./manage.py sqlmigrate
to get the created index name (it depends solely on table name and index fields name: see django/db/backends/base/schema.py:BaseDatabaseSchemaEditor._create_index_name)
comment:24 by , 6 years ago
To get the proper new index name:
from django.db import connection schema_editor = connection.schema_editor() def get_index_name(field): return schema_editor._create_index_name(field.model._meta.db_table, [field.attname]) get_index_name(ModelFoo.field_bar.field)
comment:25 by , 5 years ago
Cc: | added |
---|
comment:26 by , 5 years ago
Cc: | added |
---|
follow-up: 36 comment:27 by , 5 years ago
Description: | modified (diff) |
---|
I tried this and it works :
In the migration file I replace this :
migrations.AlterField( model_name='foo', name='variable', field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, to='app.Variable'), ),
with this :
migrations.RenameModel('Foo', 'FooNew'), migrations.AlterField( model_name='foonew', name='variable', field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, to='app.Variable'), ), migrations.RenameModel('FooNew', 'Foo'),
Is it a good solution ?
comment:29 by , 3 years ago
Just ran into this issue when attempting to rename and rebuild a table. Renaming the table doesn't change any of the indexes or constraints so I can't rebuild another table with the same ManyToMany relationship. I tried creating a through table and renaming the foreign keys but that doesn't seem to do it either as the "UNIQUE TOGETHER" index name itself never changes.
comment:30 by , 3 years ago
Cc: | added |
---|
comment:31 by , 3 years ago
Just opened this issue: https://code.djangoproject.com/ticket/33488#ticket which was apparently a duplicate of this one. I did a rename and quickly after I reused the old name of the field which made this very visible. However, I can imagine that it is a difficult error to find when it happens with longer periods in between. The longer a project lasts, the more likely it is that one will run into this kind of issues.
Unfortunately, the issue doesn't seem to have a high priority given that it has been 7 years since it was opened. For now I will sandwich the rename migration with unsetting and setting the index in two additional migrations.
comment:32 by , 2 years ago
Encountered this (django 3.2) when trying to add a field with the same name of a field I renamed in a few migrations before. The original index is still existing. It should have been renamed as part of the AlterField migration.
comment:33 by , 2 years ago
Ran into this as well, using django 3.1 and Postgres. Similar case as for scholtalbers.
I wanted to switch to using a library that wanted to add field to model which was conflicting with the existing name, used for similar purpose (parent).
I renamed the field (to old_parent), ran migration, created migration, added the library (which added the parent field), created migration, run migration.
Locally migrations worked because I was using SQLite, but when installing on staging it failed because of conflicting index name.
comment:35 by , 18 months ago
Cc: | added |
---|
comment:36 by , 18 months ago
So I ended up created a duplicate of this ticket for issue #34647 and I had a look at Clavay's suggestion but one of the issues here is that Django then detects the change in future migrations, at least if you rename the name and the db_table) and then it puts it back into a broken state as the SQL migrations that are generated are still wrong. So the work around needs to be run as raw SQL in order to bypass Django knowing about it.
comment:37 by , 16 months ago
Just hit the same problem after renaming a Model and creating a new one with the old name (to conform to how several other models in that application ended up being named).
I was able to fix it by:
- changing the new Model creation in the migration to specify
spatial_index=False
(it was a GIS field, I guess for regular ones you need to usedb_index=False
) - running makemigrations again: since I never changed the models, it detected the missing indexes and created another migration that added them to the db
This worked both for migrating the production database (was working earlier too) and when running tests.
comment:38 by , 14 months ago
I just ran into a similar Issue when reverting migrations on Postgres, specifically I have the following condensed and abstracted migration history:
- Setup:
- Create model Author
- Create model Book (
author = ForeignKey(Author)
)
- Introduce PenNames
- Create model PenName
- Alter
Book.author
to allow null - Add
author_new = ForgeignKey(PenName, null=True)
to book - RunPython to create PenNames for all authors and set
author_new
accordingly (including a corresponding reverse operation) - Drop
Book.author
- Alter
Book.author_new
to prohibit null - Rename
Book.author_new
toBook.author
- Remove PenNames (for whatever reason)
- Some Operation to repopulate
Book.author
- Drop column
Book.author
- Some Operation to repopulate
After successfully applying these migrations, reverting to State 1 will error when undoing step 2.5, since the index book_author_id...
was already created when undoing step 3.2, but it would be called book_author_new_id...
when applying these migrations forwards.
comment:39 by , 13 months ago
Owner: | set to |
---|---|
Status: | new → assigned |
I am at DjangoCon US sprints trying to get to the bottom of this
comment:40 by , 4 months ago
I encountered this issue when renaming models on Postgres. There are several indexes that need to be addressed. I'm not sure how to proceed.
I assume I can use the SQL operation, but that does not work well in SQLITE.
It is worth noting that if you use postgres instead, the error is
django.db.utils.ProgrammingError: relation "sample_foo_4350f7d0" already exists
.