Opened 9 years ago

Closed 4 years 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 4 years ago.
Test project

Download all attachments as: .zip

Change History (18)

comment:1 by Hedde van der Heide, 9 years ago

Description: modified (diff)

comment:2 by Hedde van der Heide, 9 years ago

Description: modified (diff)

comment:3 by Moritz Sichert, 9 years ago

Component: UncategorizedMigrations
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

Confirmed on master.

comment:4 by Tyson Clugg, 9 years ago

Cc: Tyson Clugg added

comment:5 by Tim Graham, 9 years ago

#26404 is a duplicate.

comment:6 by Simon Charette, 8 years ago

Keywords: postgres migrations removed
Version: 1.8master

#24954 was a duplicate for the ManyToManyField case.

comment:7 by Simon Charette, 8 years ago

#25733 was a duplicate for the OneToOneField case.

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

comment:8 by Aleksi Häkli, 8 years ago

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 by Andrew Badr, 6 years ago

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

comment:10 by Samuel Bishop, 5 years ago

Cc: Samuel Bishop added
Keywords: migrations added

comment:11 by Paul Schreiber, 4 years ago

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 by Tim Graham, 4 years ago

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

comment:13 by Rowan Seymour, 4 years ago

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 4 years ago by Rowan Seymour (previous) (diff)

comment:14 by Rowan Seymour, 4 years ago

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 by Simon Charette, 4 years ago

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 4 years ago by Simon Charette (previous) (diff)

comment:16 by Simon Charette, 4 years ago

Owner: Simon Charette removed
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.

by Simon Charette, 4 years ago

Attachment: ticket_25012.zip added

Test project

comment:17 by Mariusz Felisiak, 4 years ago

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