Opened 6 months ago

Closed 6 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)

django35022.tgz (1.1 KB ) - added by Thomas Capricelli 6 months ago.
minimal example for reproducing the bug

Download all attachments as: .zip

Change History (13)

comment:1 by Natalia Bidart, 6 months ago

Resolution: worksforme
Status: newclosed

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:

  1. Did you modified your models.py, generated the migration and migrated using the same Django version? If yes, which one?
  2. You refer to a 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.

comment:2 by Thomas Capricelli, 6 months ago

Resolution: worksforme
Status: closednew

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

by Thomas Capricelli, 6 months ago

Attachment: django35022.tgz added

minimal example for reproducing the bug

comment:3 by Mariusz Felisiak, 6 months ago

Cc: David Wobrock added
Summary: migration fails from index_together to models.IndexRenameIndex crashes when there is a unique index on the same fields.
Triage Stage: UnreviewedAccepted

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):  
    10171017                from_model._meta.get_field(field).column for field in self.old_fields
    10181018            ]
    10191019            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,
    10211021            )
    10221022            if len(matching_index_name) != 1:
    10231023                raise ValueError(

Does it work for you?

comment:4 by Thomas Capricelli, 6 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.

comment:5 by Yash Kumar Verma, 6 months ago

is someone working on a merge request for this? if not, can I try submitting this patch?

in reply to:  5 comment:6 by Mariusz Felisiak, 6 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.

in reply to:  2 comment:7 by Prakhar Parashari, 6 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 David Wobrock, 6 months ago

Has patch: set
Owner: changed from nobody to David Wobrock
Status: newassigned

comment:9 by Mariusz Felisiak, 6 months ago

Has patch: unset

comment:10 by David Wobrock, 6 months ago

Has patch: set

PR against main.

comment:11 by Mariusz Felisiak, 6 months ago

Triage Stage: AcceptedReady for checkin

comment:12 by Mariusz Felisiak <felisiak.mariusz@…>, 6 months ago

Resolution: fixed
Status: assignedclosed

In 70456610:

Fixed #35022 -- Fixed RenameIndex() crash on unnamed indexes if exists unique constraint on the same fields.

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