Opened 7 years ago

Closed 16 months ago

#25012 closed Bug (fixed)

Migrations don't make foreign key type changes

Reported by: Hedde van der Heide Owned by:
Component: Migrations Version: dev
Severity: Normal Keywords: migrations
Cc: Tyson Clugg, Samuel Bishop Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Tim Graham)

Example:

class Foo(models.Model):
    id = models.AutoField()  # Now change this to models.CharField(primary_key=True, max_length=...) and migrate, the migration doesn't complain    

class Bar(object):
    foo = models.ForeignKey(Foo)  # but Postgres will still say Bar.foo is an Integer value.
DataError at /myapp/bar/add/
invalid input syntax for integer: "TEST"
LINE 1: ...d") VALUES (NULL, 'TEST', ...

Attachments (1)

ticket_25012.zip (176 bytes) - added by Simon Charette 20 months ago.
Test project

Download all attachments as: .zip

Change History (18)

comment:1 Changed 7 years ago by Hedde van der Heide

Description: modified (diff)

comment:2 Changed 7 years ago by Hedde van der Heide

Description: modified (diff)

comment:3 Changed 7 years ago by Moritz Sichert

Component: UncategorizedMigrations
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

Confirmed on master.

comment:4 Changed 7 years ago by Tyson Clugg

Cc: Tyson Clugg added

comment:5 Changed 6 years ago by Tim Graham

#26404 is a duplicate.

comment:6 Changed 6 years ago by Simon Charette

Keywords: postgres migrations removed
Version: 1.8master

#24954 was a duplicate for the ManyToManyField case.

comment:7 Changed 6 years ago by Simon Charette

#25733 was a duplicate for the OneToOneField case.

Last edited 6 years ago by Simon Charette (previous) (diff)

comment:8 Changed 6 years ago by Aleksi Häkli

I was able to neatly fix this with a raw SQL migration that changed the intermediary table column type.

I had the following schema in an application called documents:

class Tag(models.Model):
    # converted this `name` field to primary_key
    # previously had the default `id` AutoField as primary_key field
    # PrimaryKeyField migration in for ManyToManyField is risky!
    name = models.CharField(max_length=32, primary_key=True)

class Document(models.Model):
    tags = models.ManyToManyField(Tag, blank=True)

After changing the schema and running unit tests I discovered that I indeed to got a DatabaseError:

django.db.utils.DataError: invalid input syntax for integer: "Miss Amanda Goodwin"
LINE 1: ..."tag_id") WHERE "documents_document_tags"."tag_id" = 'Miss Aman...

Migrating with the following helped:

ALTER TABLE documents_document_tags ALTER COLUMN tag_id TYPE varchar(32);

This of course had to be added as a migration file for sane functionality in tests and migration chain:

# -*- coding: utf-8 -*-
# Migrations file example for fixing broken ManyToManyField migration from INTEGER to VARCHAR
# Generated by Django 1.10.3 on 1970-01-01 00:00
from __future__ import unicode_literals

from django.db import migrations

class Migration(migrations.Migration):
    dependencies = [
        ('documents', '0042_auto_19700101-0000'),
    ]

    operations = [
        migrations.RunSQL('ALTER TABLE documents_document_tags ALTER tag_id TYPE varchar(32);'),
    ]

comment:9 Changed 4 years ago by Andrew Badr

If this is the same as #29787, then 45ded053b1f4320284aa5dac63052f6d1baefea9 and b8a2f3c2d66aa15af4be745a576609b958a853c0 might be useful commits to look at.

comment:10 Changed 3 years ago by Samuel Bishop

Cc: Samuel Bishop added
Keywords: migrations added

comment:11 Changed 20 months ago by Paul Schreiber

I ran in to this today (Django 3.1.4). The workaround was to

UPDATE myapp.mytable set my_fk_column = NULL;

and re-run the migration.

comment:12 Changed 20 months ago by Tim Graham

Description: modified (diff)
Summary: Migration doesn't seem to detect foreignKey type changesMigrations don't make foreign key type changes

comment:13 Changed 20 months ago by Rowan Seymour

Ran into this migrating primary keys to BigAutoField. Was expecting it to generate explicit migrations for changes to foreign keys to those ids, but it doesn't. Running sqlmigrate also didn't show any ALTER operations to foreign keys. However running the migration for the BigAutoField change, resulted in an ALTER on one foreign key in a different app, but not another in the same app.

Last edited 20 months ago by Rowan Seymour (previous) (diff)

comment:14 Changed 20 months ago by Rowan Seymour

Having experimented a bit more - it seems changing a primary key to BigAutoField does change any foreign keys to BIGINT when the migration is applied - except in implicit M2M through models. Those remain as INT and have to be fixed manually.

comment:15 Changed 20 months ago by Simon Charette

Owner: changed from nobody to Simon Charette
Status: newassigned

I had a look at the field alteration logic and it seems to come from the fact m2m relationships are excluded from this block

https://github.com/django/django/blob/415f50298f97fb17f841a9df38d995ccf347dfcc/django/db/backends/base/schema.py#L793-L808

I've assigned it to myself as it's likely something we want to get fixed before the final 3.2 release which introduces warnings about AutoField.

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

comment:16 Changed 20 months ago by Simon Charette

Owner: Simon Charette deleted
Status: assignednew

I managed to reproduce but I think the issue is specific to sqlmigrate when dealing with cross-app relationships and due to how related models are lazily built. When using the migrate command everything seems to work fine.

The alter_field method relies on _related_non_m2m_objects to determine which models points back at the field being altered but since sqlmigrate only loads the minimal plan it might not render some back referencing models and thus _related_non_m2m_objects won't return them. Unless someone can reproduce the same issue by solely using migrate I think this issue should be closed as fixed since we already tests for it and a new one could be opened for sqlmigrate misbehaviour.

I attached a test project I used to reproduce the sqlmigrate issue and confirm migrate works fine. Notice that sqlmigrate app_one 0002 doesn't mention app_two_baz_foos at all unless app_one.0002_auto_20210105_2317 is changed to depend on app_two.0001_initial which forces the rendering of the app_two.Baz.Baz_Foo model. I suspect the issue can also be reproduced using a ForeignKey across apps.

Changed 20 months ago by Simon Charette

Attachment: ticket_25012.zip added

Test project

comment:17 Changed 16 months ago by Mariusz Felisiak

Resolution: fixed
Status: newclosed

I agree with Simon. I've confirmed that this's already fixed with a separate sample project. Also, I cannot reproduce an issue with sqlmigrate.

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