[Django] #29123: Generated migration orders Add/Remove Field incorrectly, causing OperationalError
Django
OperationalError
--------------------------------------+------------------------
Reporter: Ed Morley | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
--------------------------------------+------------------------
Replacing an integer field with a foreign key of the same name, results in
an `OperationalError` when creating/applying the migration.
This affects Django master + v1.11.10, and both the SQLite and MySQL
backends (others not tested).
== STR:
1. Git clone `https://github.com/edmorley/django-migration-int-to-fk-
testcase`
2. `pip install https://github.com/django/django/archive/master.zip`
3. `./manage.py migrate`
4. `cp testapp/models_new.py testapp/models.py`
5. `./manage.py makemigrations --name broken_migration`
6. `./manage.py migrate`
== Expected:
New migration is created/applied successfully, which converts from the
[https://github.com/edmorley/django-migration-int-to-fk-
testcase/blob/master/testapp/models.py original model] to the
[https://github.com/edmorley/django-migration-int-to-fk-
testcase/blob/master/testapp/models_new.py new model].
== Actual:
The new `0002_broken_migration.py` migration incorrectly lists the
`AddField`
operation before the `RemoveField` operation...
{{{#!python
operations = [
migrations.AddField(
model_name='bar',
name='foo',
field=models.ForeignKey(null=True,
on_delete=django.db.models.deletion.CASCADE, to='testapp.Foo'),
),
migrations.RemoveField(
model_name='bar',
name='foo_id',
),
migrations.AlterUniqueTogether(
name='bar',
unique_together={('name', 'foo')},
),
]
}}}
Which results in an exception at step 6...
{{{#!bash
$ ./manage.py migrate
Operations to perform:
Apply all migrations: admin, auth, contenttypes, sessions, testapp
Running migrations:
Applying testapp.0002_broken_migration...Traceback (most recent call
last):
File "/c/Users/Ed/.virtualenvs/django-master/lib/python3.6/site-
packages/django/db/backends/utils.py", line 83, in _execute
return self.cursor.execute(sql)
File "/c/Users/Ed/.virtualenvs/django-master/lib/python3.6/site-
packages/django/db/backends/sqlite3/base.py", line 290, in execute
return Database.Cursor.execute(self, query)
sqlite3.OperationalError: duplicate column name: foo_id
}}}
== Additional notes:
* Without the `unique_together` on model `Bar`, the bug does not occur.
* This affects both the SQLite backend and the MySQL backend (others not
tested).
* At time of testing, django master was at revision
`6d794fb76212bb8a62fe2cd97cff173054e1c626`.
--
Ticket URL: <https://code.djangoproject.com/ticket/29123>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
OperationalError
Reporter: Ed Morley | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Ed Morley:
Old description:
New description:
Replacing an integer field with a foreign key of the same name, results in
an `OperationalError` when creating/applying the migration.
This affects Django master + v1.11.10, and both the SQLite and MySQL
backends (others not tested).
== STR:
1. Git clone [https://github.com/edmorley/django-migration-int-to-fk-
testcase https://github.com/edmorley/django-migration-int-to-fk-testcase]
== Expected:
== Actual:
File ".../site-packages/django/db/backends/utils.py", line 83, in
_execute
return self.cursor.execute(sql)
File ".../site-packages/django/db/backends/sqlite3/base.py", line 290,
in execute
return Database.Cursor.execute(self, query)
sqlite3.OperationalError: duplicate column name: foo_id
}}}
== Additional notes:
* Without the `unique_together` on model `Bar`, the bug does not occur.
* This affects both the SQLite backend and the MySQL backend (others not
tested).
* At time of testing, django master was at revision
`6d794fb76212bb8a62fe2cd97cff173054e1c626`.
* The above was using Python 3.6.2.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/29123#comment:1>
Django
migration operations if the field is in unique_together
----------------------------+------------------------------------
Reporter: Ed Morley | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* stage: Unreviewed => Accepted
Comment:
I didn't attempt to reproduce but the report looks credible.
--
Ticket URL: <https://code.djangoproject.com/ticket/29123#comment:2>
Django
migration operations if the field is in unique_together
Reporter: Ed Morley | Owner: Bhavesh Praveen
Type: Bug | Status: assigned
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Bhavesh Praveen):
* owner: nobody => Bhavesh Praveen
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/29123#comment:3>
Django
migration operations if the field is in unique_together
Reporter: Ed Morley | Owner: (none)
Type: Bug | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Bhavesh Praveen):
* status: assigned => new
* owner: Bhavesh Praveen => (none)
--
Ticket URL: <https://code.djangoproject.com/ticket/29123#comment:4>
Django
migration operations if the field is in unique_together
----------------------------+------------------------------------
Type: Bug | Status: assigned
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jeff):
* owner: nobody => Jeff
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/29123#comment:3>
Django
migration operations if the field is in unique_together
----------------------------+------------------------------------
Reporter: Ed Morley | Owner: Jeff
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Jeff):
I was able to reproduce and am starting work.
--
Ticket URL: <https://code.djangoproject.com/ticket/29123#comment:4>
Django
migration operations if the field is in unique_together
----------------------------+------------------------------------
Reporter: Ed Morley | Owner: Jeff
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Jeff):
It looks like the example migrations provided are exposing two separate
issues.
1) improperly ordered AddField/RemoveField/AlterUniqueTogether. Below is
the initial migration
Original Models:
{{{
class Foo(models.Model):
pass
class Bar(models.Model):
name = models.CharField(max_length=50)
foo_id = models.PositiveIntegerField()
class Meta:
unique_together = ('name', 'foo_id')
}}}
Updated Models:
{{{
class Foo(models.Model):
pass
class Bar(models.Model):
name = models.CharField(max_length=
baz = models.ForeignKey(Foo, models.CASCADE, null=True)
class Meta:
unique_together = ('name', 'baz')
}}}
This produces the problematic migration:
{{{
class Migration(migrations.Migration):
dependencies = [
('temp', '0001_initial'),
]
operations = [
migrations.AddField(
model_name='bar',
name='baz',
field=models.ForeignKey(null=True,
on_delete=django.db.models.deletion.CASCADE, to='temp.Foo'),
),
migrations.RemoveField(
model_name='bar',
name='foo_id',
),
migrations.AlterUniqueTogether(
name='bar',
unique_together={('name', 'baz')},
),
]
}}}
Instead of this functional migration:
{{{
class Migration(migrations.Migration):
dependencies = [
('temp', '0001_initial'),
]
operations = [
migrations.AddField(
model_name='bar',
name='baz',
field=models.ForeignKey(null=True,
on_delete=django.db.models.deletion.CASCADE, to='temp.Foo'),
),
migrations.AlterUniqueTogether(
name='bar',
unique_together={('name', 'baz')},
),
migrations.RemoveField(
model_name='bar',
name='foo_id',
),
]
}}}
and 2) problematic behavior around the clashing of names like `foo` with
`foo_id`.
I believe the given examples somewhat conflated these two separate issues,
while I believe 1) is a bug and should be fixed, the I believe 2) is
working as intended.
Please advise if I am incorrect on this assumption.
--
Ticket URL: <https://code.djangoproject.com/ticket/29123#comment:5>
Django
migration operations if the field is in unique_together
----------------------------+------------------------------------
Reporter: Ed Morley | Owner: Jeff
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Jeff):
I have tracked down the issue to the migration optimization. Debugging now
to determine how to ensure a field cannot be removed while it is a part of
a `unique_together` constraint.
--
Ticket URL: <https://code.djangoproject.com/ticket/29123#comment:6>
Django
migration operations if the field is in unique_together
----------------------------+------------------------------------
Reporter: Ed Morley | Owner: Jeff
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Ramiro Morales):
Looks suspiciously similar or at least related to #28862.
--
Ticket URL: <https://code.djangoproject.com/ticket/29123#comment:7>
Django
migration operations if the field is in unique_together
----------------------------+------------------------------------
Reporter: Ed Morley | Owner: Jeff
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Jeff):
Thanks for the tip, I do believe it to be the same issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/29123#comment:8>
Django
migration operations if the field is in unique_together
----------------------------+------------------------------------
Type: Bug | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jeff):
* owner: Jeff => (none)
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/29123#comment:9>
Django
migration operations if the field is in unique_together
----------------------------+------------------------------------
Reporter: Ed Morley | Owner: (none)
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Jeff):
@TimGraham I believe this can be closed as a duplicate of #28862
--
Ticket URL: <https://code.djangoproject.com/ticket/29123#comment:10>
Django
migration operations if the field is in unique_together
Reporter: Ed Morley | Owner: (none)
Component: Migrations | Version: master
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Ramiro Morales):
* status: new => closed
* resolution: => duplicate
Comment:
Duplicate of #28862
--
Ticket URL: <https://code.djangoproject.com/ticket/29123#comment:11>