Opened 3 years ago

Last modified 8 days ago

#23577 new Bug

Rename operations should rename indexes, constraints, sequences and triggers named after their former value

Reported by: Chris Woytowitz Owned by:
Component: Migrations Version: master
Severity: Normal Keywords:
Cc: bugs@…, aksheshdoshi@…, emorley@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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:

  1. Create a model Bar.
  2. Create a model Foo with bar = models.ForeignKey(Bar, blank=True, null=True)
  3. makemigrations (migration 0001)
  4. Rename Foo to OldFoo.
  5. makemigrations, say yes to the rename prompt (migration 0002)
  6. Create new model Foo, which also has bar = models.ForeignKey(Bar, blank=True, null=True)
  7. makemigrations (migration 0003)
  8. 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)

dj17test.zip (12.1 KB) - added by Chris Woytowitz 3 years ago.

Download all attachments as: .zip

Change History (19)

Changed 3 years ago by Chris Woytowitz

Attachment: dj17test.zip added

comment:1 Changed 3 years ago by Chris Woytowitz

It is worth noting that if you use postgres instead, the error is django.db.utils.ProgrammingError: relation "sample_foo_4350f7d0" already exists.

comment:2 Changed 3 years ago by Simon Charette

Triage Stage: UnreviewedAccepted

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 Changed 3 years ago by Tom V

Owner: changed from nobody to Tom V
Status: newassigned

comment:4 Changed 3 years ago by Tom V

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 Changed 3 years ago by Tom V

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.

Last edited 3 years ago by Tom V (previous) (diff)

comment:6 Changed 3 years ago by Tom V

Owner: Tom V deleted
Status: assignednew

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 Changed 3 years ago by ris

Cc: bugs@… added

comment:8 in reply to:  4 ; Changed 3 years ago by 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.

comment:9 Changed 2 years ago by skyjur

This also happens when renaming a ManyToManyField and then adding a new ManyToManyField with the same name. I've opened ticket #25752 then @timgraham pointed out that it's duplicate of this one.

Last edited 2 years ago by Tim Graham (previous) (diff)

comment:10 Changed 23 months ago by Ian Clark

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 in reply to:  8 Changed 22 months ago by Luke Murphy

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 Changed 21 months ago by Akshesh Doshi

Cc: aksheshdoshi@… added

comment:13 Changed 18 months ago by Simon Charette

Summary: CREATE INDEX-related OperationalError on migrating renamed models with colliding field namesRenameModel should rename indexes and constraints named after its old table name
Version: 1.7master

#24528 was a duplicate.

comment:14 Changed 18 months ago by Simon Charette

Summary: RenameModel should rename indexes and constraints named after its old table nameRenameModel 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.

Last edited 18 months ago by Simon Charette (previous) (diff)

comment:15 Changed 18 months ago by Simon Charette

Summary: RenameModel should rename indexes, constraints, sequences and triggers named after its old table nameRename 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 Changed 14 months ago by Ed Morley

Cc: emorley@… added

comment:17 Changed 7 weeks ago by Martin Koistinen

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:18 Changed 8 days ago by Tim Graham

I closed #28803 as a duplicate.

Note: See TracTickets for help on using tickets.
Back to Top