Opened 11 months ago
Closed 11 months ago
#35022 closed Bug (fixed)
RenameIndex crashes when there is a unique index on the same fields.
Reported by: | Thomas Capricelli | Owned by: | David Wobrock |
---|---|---|---|
Component: | Migrations | Version: | 5.0 |
Severity: | Normal | Keywords: | |
Cc: | David Wobrock | 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
As index_together is obsolated, i changed in my models.py:
- index_together = [ - ('name', 'host',) - ] + indexes = [models.Index(fields=["name", "host"])]
makemigrations created what seems a legit migration:
# Generated by Django 5.0.1 on 2023-12-06 12:46 from django.db import migrations class Migration(migrations.Migration): dependencies = [ ('main', '0004_auto_20200327_0040'), ] operations = [ migrations.RenameIndex( model_name='list', new_name='main_list_name_cbdf64_idx', old_fields=('name', 'host'), ), ]
But the actual migration fails badly:
% ./manage.py migrate Operations to perform: Apply all migrations: admin, auth, contenttypes, flatpages, main, registration, sessions, sites Running migrations: Applying main.0005_rename_list_name_host_main_list_name_cbdf64_idx... Traceback (most recent call last): File "/home/orzel/hg/xxx/./manage.py", line 8, in <module> execute_from_command_line(sys.argv) File "/usr/lib/python3.11/site-packages/django/core/management/__init__.py", line 442, in execute_from_command_line utility.execute() File "/usr/lib/python3.11/site-packages/django/core/management/__init__.py", line 436, in execute self.fetch_command(subcommand).run_from_argv(self.argv) File "/usr/lib/python3.11/site-packages/django/core/management/base.py", line 412, in run_from_argv self.execute(*args, **cmd_options) File "/usr/lib/python3.11/site-packages/django/core/management/base.py", line 458, in execute output = self.handle(*args, **options) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.11/site-packages/django/core/management/base.py", line 106, in wrapper res = handle_func(*args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.11/site-packages/django/core/management/commands/migrate.py", line 356, in handle post_migrate_state = executor.migrate( ^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.11/site-packages/django/db/migrations/executor.py", line 135, in migrate state = self._migrate_all_forwards( ^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.11/site-packages/django/db/migrations/executor.py", line 167, in _migrate_all_forwards state = self.apply_migration( ^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.11/site-packages/django/db/migrations/executor.py", line 252, in apply_migration state = migration.apply(state, schema_editor) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.11/site-packages/django/db/migrations/migration.py", line 132, in apply operation.database_forwards( File "/usr/lib/python3.11/site-packages/django/db/migrations/operations/models.py", line 1050, in database_forwards raise ValueError( ValueError: Found wrong number (2) of indexes for main_list(name, host).
I'm currently testing and the database is sqlite3. Browsing the sqlite file i can see 3 indices on this table: one unrelated (id) and two for name_host : one "idx" and one "uniq".
My bet is that having both an index_together and a unique_together is ... bad ? (my bad?).
Or maybe it just freaks out django ? It might be specific to sqlite.
Attachments (1)
Change History (13)
comment:1 by , 11 months ago
Resolution: | → worksforme |
---|---|
Status: | new → closed |
follow-up: 7 comment:2 by , 11 months ago
Resolution: | worksforme |
---|---|
Status: | closed → new |
I created a minimal exemple. All of this is done using django 4.2 (but i'm pretty sure it does the same with django 5.0):
How to use:
./manage.py makemigrations main && ./manage.py migrate # create initial migrations
vi main/models.py # comment index_together, uncomment indexes =
./manage.py makemigrations main && ./manage.py migrate # crash
comment:3 by , 11 months ago
Cc: | added |
---|---|
Summary: | migration fails from index_together to models.Index → RenameIndex crashes when there is a unique index on the same fields. |
Triage Stage: | Unreviewed → Accepted |
Thanks for the report. The following patch fixes this for me:
-
django/db/migrations/operations/models.py
diff --git a/django/db/migrations/operations/models.py b/django/db/migrations/operations/models.py index d616cafb45..204174f911 100644
a b class RenameIndex(IndexOperation): 1017 1017 from_model._meta.get_field(field).column for field in self.old_fields 1018 1018 ] 1019 1019 matching_index_name = schema_editor._constraint_names( 1020 from_model, column_names=columns, index=True 1020 from_model, column_names=columns, index=True, unique=False, 1021 1021 ) 1022 1022 if len(matching_index_name) != 1: 1023 1023 raise ValueError(
Does it work for you?
comment:4 by , 11 months ago
I dont understand fully what's happening, but yes, it fixes it for me. Congrats !
I tested with django 5.0 and the line in the file was slightly different.
follow-up: 6 comment:5 by , 11 months ago
is someone working on a merge request for this? if not, can I try submitting this patch?
comment:6 by , 11 months ago
Replying to Yash Kumar Verma:
is someone working on a merge request for this? if not, can I try submitting this patch?
Yes you can, please remember that a regression test is required.
comment:7 by , 11 months ago
Replying to Thomas Capricelli:
I created a minimal exemple. All of this is done using django 4.2 (but i'm pretty sure it does the same with django 5.0):
How to use:
./manage.py makemigrations main && ./manage.py migrate # create initial migrations
vi main/models.py # comment index_together, uncomment indexes =
./manage.py makemigrations main && ./manage.py migrate # crash
Hey, I tried to reproduce this with your minimal example with Django 5.0 and Django 4.2. However, I couldn't reproduce it. Maybe the example file is missing something that was producing this error for you? I'm not sure
comment:8 by , 11 months ago
Has patch: | set |
---|---|
Owner: | changed from | to
Status: | new → assigned |
Opened PRs for 5.0 https://github.com/django/django/pull/17632 and 4.2 https://github.com/django/django/pull/17633 using Mariusz's fix :)
comment:9 by , 11 months ago
Has patch: | unset |
---|
comment:11 by , 11 months ago
Triage Stage: | Accepted → Ready for checkin |
---|
Hello, thank you for your report.
I have tried to reproduce this with a minimal example using the data you showed above, but I can't reproduce (I used Django 4.2 since that's the version where the deprecation started, and I also tested on Django 5.0). Questions:
unique_together
index which you haven't provided any details for, could you please share minimal but complete models.py to reproduce the issue?Lastly, if you are unsure about whether your models are doing the right thing or not, you could also seek assistance from one of the Django support channels where there are many friendly people that can help you: https://www.djangoproject.com/community/
Closing as
worksforme
for now, until further details are provided.