[Django] #25012: Migration doesn't seem to detect foreignKey type changes
Django
-------------------------------+----------------------------------
Reporter: Hedde | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.8
Severity: Normal | Keywords: postgres, migrations
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+----------------------------------
Example:
{{{
#!div style="font-size: 80%"
Code highlighting:
{{{#!python
class Foo(models.Model):
id = models.AutoField() # Now change this to
models.CharField(primary_key=True) 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.
}}}
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25012>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Hedde | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.8
Keywords: postgres, | Triage Stage:
migrations | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Hedde):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Old description:
> Example:
>
> {{{
> #!div style="font-size: 80%"
> Code highlighting:
> {{{#!python
> class Foo(models.Model):
> id = models.AutoField() # Now change this to
> models.CharField(primary_key=True) 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.
> }}}
> }}}
New description:
Example:
{{{
#!div style="font-size: 80%"
{{{#!python
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.
}}}
}}}
--
--
Ticket URL: <https://code.djangoproject.com/ticket/25012#comment:1>
Django
Reporter: Hedde | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.8
Keywords: postgres, | Triage Stage:
migrations | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Hedde:
Old description:
> Example:
>
> {{{
> #!div style="font-size: 80%"
> {{{#!python
> 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.
> }}}
> }}}
New description:
Example:
{{{
#!div style="font-size: 80%"
{{{#!python
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', ...
--
--
Ticket URL: <https://code.djangoproject.com/ticket/25012#comment:2>
Django
Reporter: Hedde | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: postgres, migrations | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by MoritzS):
* type: Uncategorized => Bug
* component: Uncategorized => Migrations
* stage: Unreviewed => Accepted
Comment:
Confirmed on master.
--
Ticket URL: <https://code.djangoproject.com/ticket/25012#comment:3>
Django
Reporter: Hedde | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: postgres, migrations | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by tysonclugg):
* cc: tysonclugg (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/25012#comment:4>
Django
Reporter: Hedde | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.8
Severity: Normal | Resolution:
Keywords: postgres, migrations | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
#26404 is a duplicate.
--
Ticket URL: <https://code.djangoproject.com/ticket/25012#comment:5>
Django
Reporter: Hedde | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by charettes):
* keywords: postgres, migrations =>
* version: 1.8 => master
Comment:
#24954 was a duplicate for the `ManyToManyField` case.
--
Ticket URL: <https://code.djangoproject.com/ticket/25012#comment:6>
Django
Reporter: Hedde | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by charettes):
#25733 was a duplicate for `OneToOneField` case.
--
Ticket URL: <https://code.djangoproject.com/ticket/25012#comment:7>
Django
Reporter: Hedde van der Heide | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (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);'),
]
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25012#comment:8>
Django
Reporter: Hedde van der Heide | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Andrew Badr):
If this is the same as #29787, then
45ded053b1f4320284aa5dac63052f6d1baefea9 and
b8a2f3c2d66aa15af4be745a576609b958a853c0 might be useful commits to look
at.
--
Ticket URL: <https://code.djangoproject.com/ticket/25012#comment:9>
Django
Reporter: Hedde van der Heide | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Samuel Bishop):
* keywords: => migrations
* cc: Samuel Bishop (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/25012#comment:10>
Django
Reporter: Hedde van der Heide | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: migrations | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (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.
--
Ticket URL: <https://code.djangoproject.com/ticket/25012#comment:11>
Django
-------------------------------------+------------------------------------
Reporter: Hedde van der Heide | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: migrations | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Tim Graham:
Old description:
> Example:
>
> {{{
> #!div style="font-size: 80%"
> {{{#!python
> 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', ...
New description:
Example:
{{{#!python
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', ...
}}}
--
--
Ticket URL: <https://code.djangoproject.com/ticket/25012#comment:12>
Django
-------------------------------------+------------------------------------
Reporter: Hedde van der Heide | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: migrations | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (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.
--
Ticket URL: <https://code.djangoproject.com/ticket/25012#comment:13>
Django
-------------------------------------+------------------------------------
Reporter: Hedde van der Heide | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: migrations | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (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.
--
Ticket URL: <https://code.djangoproject.com/ticket/25012#comment:14>
Django
Reporter: Hedde van der Heide | Owner: Simon
| Charette
Type: Bug | Status: assigned
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: migrations | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Charette):
* owner: nobody => Simon Charette
* status: new => assigned
Comment:
I had a look at the field alteration logic and it seems to come from the
fact m2m relationships are excluded from this block
I've assigned it to myself as it's likely something we want to get fix
before the final 3.2 release which introduces warnings about `AutoField`.
--
Ticket URL: <https://code.djangoproject.com/ticket/25012#comment:15>
Django
-------------------------------------+------------------------------------
Type: Bug | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: migrations | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Charette):
* owner: Simon Charette => (none)
* status: assigned => new
Comment:
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
[https://github.com/django/django/blob/415f50298f97fb17f841a9df38d995ccf347dfcc/tests/migrations/test_operations.py#L1434-L1494
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/25012#comment:16>
Django
-------------------------------------+------------------------------------
Reporter: Hedde van der Heide | Owner: (none)
Type: Bug | Status: new
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: migrations | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Charette):
* Attachment "ticket_25012.zip" added.
Test project
Django
-------------------------------------+------------------------------------
Reporter: Hedde van der Heide | Owner: (none)
Component: Migrations | Version: dev
Severity: Normal | Resolution: fixed
Keywords: migrations | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* status: new => closed
* resolution: => fixed
Comment:
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`.
--
Ticket URL: <https://code.djangoproject.com/ticket/25012#comment:17>