Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#22476 closed Bug (fixed)

Migrations fail to migrate m2m fields with custom through model.

Reported by: Florian Apolloner Owned by: Andrew Godwin
Component: Migrations Version: master
Severity: Release blocker Keywords: migrations, m2m
Cc: Andrew Godwin, Florian Apolloner, k@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Changing blank and other attributes on a m2m field fails, models:

from django.db import models


class Table1(models.Model):
    pass

class Table2(models.Model):
    m2m = models.ManyToManyField(Table1, through='M2M')

class M2M(models.Model):
    t1 = models.ForeignKey(Table1, related_name='+')
    t2 = models.ForeignKey(Table2, related_name='+')

Reproduce by running makemigrations, changing blank to True on the m2m field, rerunning makemigrations and migrate, resulting in:

Operations to perform:
  Synchronize unmigrated apps: admin, contenttypes, auth, sessions
  Apply all migrations: t1
Synchronizing apps without migrations:
  Creating tables...
    Creating table django_admin_log
    Creating table auth_permission
    Creating table auth_group_permissions
    Creating table auth_group
    Creating table auth_user_groups
    Creating table auth_user_user_permissions
    Creating table auth_user
    Creating table django_content_type
    Creating table django_session
  Installing custom SQL...
  Installing indexes...
Running migrations:
  Applying t1.0001_initial... OK
  Applying t1.0002_m2m_t2... OK
  Applying t1.0003_auto_20140419_1201...Traceback (most recent call last):
  File "./manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/home/florian/sources/django.git/django/core/management/__init__.py", line 427, in execute_from_command_line
    utility.execute()
  File "/home/florian/sources/django.git/django/core/management/__init__.py", line 419, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/home/florian/sources/django.git/django/core/management/base.py", line 288, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/home/florian/sources/django.git/django/core/management/base.py", line 337, in execute
    output = self.handle(*args, **options)
  File "/home/florian/sources/django.git/django/core/management/commands/migrate.py", line 145, in handle
    executor.migrate(targets, plan, fake=options.get("fake", False))
  File "/home/florian/sources/django.git/django/db/migrations/executor.py", line 60, in migrate
    self.apply_migration(migration, fake=fake)
  File "/home/florian/sources/django.git/django/db/migrations/executor.py", line 94, in apply_migration
    migration.apply(project_state, schema_editor)
  File "/home/florian/sources/django.git/django/db/migrations/migration.py", line 97, in apply
    operation.database_forwards(self.app_label, schema_editor, project_state, new_state)
  File "/home/florian/sources/django.git/django/db/migrations/operations/fields.py", line 130, in database_forwards
    schema_editor.alter_field(from_model, from_field, to_field)
  File "/home/florian/sources/django.git/django/db/backends/sqlite3/schema.py", line 151, in alter_field
    new_field,
ValueError: Cannot alter field t1.Table2.m2m into t1.Table2.m2m - they are not compatible types (probably means only one is an M2M with implicit through model)

Marking as release blocker since this is a bug in a new feature

Change History (10)

comment:1 Changed 2 years ago by Florian Apolloner

Cc: Florian Apolloner added

comment:2 Changed 2 years ago by Tim Graham

Triage Stage: UnreviewedAccepted

comment:3 Changed 2 years ago by Andrew Godwin

I say this is valid behaviour - as the error message says, you cannot alter an M2M without a through model into one with a through model, at least not in the current system.

I propose the only change we make is that the autodetector raises the error when creating the migrations rather than it happening at runtime, and suggests that you can remove and add the field instead if you really want to drop all data and change it.

comment:4 Changed 2 years ago by Kevin Christopher Henry

Cc: k@… added

I can reproduce this, and the M2M uses a through model both before and after. The only difference is the addition of blank=True. My understanding was that blank is used for form validation and doesn't affect the database at all. Am I missing something?

comment:5 Changed 2 years ago by Andrew Godwin

Hm, that should be allowable - you're getting the exact same traceback as in the ticket description?

comment:6 Changed 2 years ago by Kevin Christopher Henry

Yes, it is exactly the same traceback (other than the model / field names, since I'm seeing this on an actual project). This is after making the lone blank=True change, and getting this single operation in the migrations file generated by makemigrations:

operations = [
        migrations.AlterField(
            model_name='question',
            name='tags',
            field=models.ManyToManyField(to=u'questions.Tag', through=u'questions.ContentTag', blank=True),
        ),
    ]

comment:7 Changed 2 years ago by Andrew Godwin

Alright, I'll make sure I cover that too; I suspect there's an easy fix where I can just ignore if the to and through properties match.

comment:8 Changed 2 years ago by Andrew Godwin

Owner: changed from nobody to Andrew Godwin
Status: newassigned

comment:9 Changed 2 years ago by Andrew Godwin <andrew@…>

Resolution: fixed
Status: assignedclosed

In b25aee3b7bef07dd07bed2209efcdf1a8146112c:

Fixed #22476: Couldn't alter attributes on M2Ms with through= set

comment:10 Changed 2 years ago by Andrew Godwin <andrew@…>

In f2bf59a5bc373eeb60136f3b3764e156efbc1ea8:

[1.7.x] Fixed #22476: Couldn't alter attributes on M2Ms with through= set

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