Opened 2 years ago
Closed 23 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 , 2 years ago
| Resolution: | → worksforme |
|---|---|
| Status: | new → closed |
follow-up: 7 comment:2 by , 2 years 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 , 2 years 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 , 2 years 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 , 2 years ago
is someone working on a merge request for this? if not, can I try submitting this patch?
comment:6 by , 2 years 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 , 2 years 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 , 23 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 , 23 months ago
| Has patch: | unset |
|---|
comment:11 by , 23 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_togetherindex 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
worksformefor now, until further details are provided.