[Django] #31317: Migration using CreateModel with unique_together followed by AlterUniqueTogether crashes
Django
AlterUniqueTogether crashes
-------------------------------------------------+------------------------
Reporter: Adam (Chainz) Johnson | 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 |
-------------------------------------------------+------------------------
Full project to reproduce: https://github.com/adamchainz/django-unique-
together-bug
When squashing migrations I ended up with a migration that added a model
with `unique_together`, then altered the unique_together later:
{{{
migrations.CreateModel(
name="Book",
fields=[
(
"id",
models.AutoField(
auto_created=True,
primary_key=True,
serialize=False,
verbose_name="ID",
),
),
("title", models.CharField(default="", max_length=100)),
],
options={"unique_together": {("title",)},},
),
# ...
migrations.AlterUniqueTogether(
name="book", unique_together={("title", "author")},
),
}}}
This can crash when using `sqlmigrate` or `migrate` (see repo) with:
{{{
File "/.../django/django/db/backends/base/schema.py", line 380, in
alter_unique_together
self._delete_composed_index(model, fields, {'unique': True},
self.sql_delete_unique)
File "/.../django/django/db/backends/base/schema.py", line 414, in
_delete_composed_index
", ".join(columns),
ValueError: Found wrong number (0) of constraints for testapp_book(title)
}}}
This is because `_delete_composed_index` from `AlterUniqueTogehter` tries
to find the existing constraint name from the database.. But it hasn't
been created yet, so it could never be found - its SQL is sitting in
`deferred_sql`.
--
Ticket URL: <https://code.djangoproject.com/ticket/31317>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
AlterUniqueTogether crashes
Reporter: Adam (Chainz) | Owner: nobody
Johnson |
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
Comment (by Simon Charette):
Which version of Django did you use to squash the migrations? A
`[CreateModel, AlterUniqueTogether]` sequence should get optimized into
`[CreateModel]`
--
Ticket URL: <https://code.djangoproject.com/ticket/31317#comment:1>
Django
AlterUniqueTogether crashes
Reporter: Adam (Chainz) | Owner: nobody
Johnson |
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
Comment (by Adam (Chainz) Johnson):
2.2, 3.0, master
It isn't squashed in this case because the `unique_together` includes a
FK.
https://github.com/adamchainz/django-unique-together-
bug/blob/master/testproj/testapp/migrations/0001_squashed_0002_add_author.py
{{{#!python
# Generated by Django 2.1.15 on 2020-02-28 06:56
from django.db import migrations, models
import django.db.models.deletion
class Migration(migrations.Migration):
replaces = [("testapp", "0001_initial"), ("testapp",
"0002_add_author")]
initial = True
dependencies = []
operations = [
migrations.CreateModel(
name="Book",
fields=[
(
"id",
models.AutoField(
auto_created=True,
primary_key=True,
serialize=False,
verbose_name="ID",
),
),
("title", models.CharField(default="", max_length=100)),
],
options={"unique_together": {("title",)},},
),
migrations.CreateModel(
name="Author",
fields=[
(
"id",
models.AutoField(
auto_created=True,
primary_key=True,
serialize=False,
verbose_name="ID",
),
),
],
),
migrations.AddField(
model_name="book",
name="author",
field=models.ForeignKey(
null=True,
on_delete=django.db.models.deletion.DO_NOTHING,
to="testapp.Author",
),
),
migrations.AlterUniqueTogether(
name="book", unique_together={("title", "author")},
),
]
}}}
It's also possible to hand write such a migration.
--
Ticket URL: <https://code.djangoproject.com/ticket/31317#comment:2>
Django
AlterUniqueTogether crashes
Reporter: Adam (Chainz) | Owner: nobody
Johnson |
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 Simon Charette):
* stage: Unreviewed => Accepted
Comment:
We'll likely have to use a specialized `ddl_statement.Statement` subclass
when deferring constraints and indices and have `_delete_composed_index`
consider them by looping through `dereferred_sql` before raising that
exception.
That would have the benefit of also avoiding creating constraints meant to
be immediately dropped.
--
Ticket URL: <https://code.djangoproject.com/ticket/31317#comment:3>
Django
AlterUniqueTogether crashes
Reporter: Adam (Chainz) | Owner: nobody
Johnson |
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Adam (Chainz) Johnson):
An alternative would be to translate `unique_together = [['a']]` into an
extra constraint in `constraints`` and then it can be treated by
migrations as `AddConstraint` / `RemoveConstraint` operations. Though that
might mean more work.
--
Ticket URL: <https://code.djangoproject.com/ticket/31317#comment:4>
Django
AlterUniqueTogether crashes
Reporter: Adam (Chainz) | Owner: nobody
Johnson |
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Simon Charette):
That approach would also not account for constraint name instabilities
that shipped over the past years in various Django versions.
--
Ticket URL: <https://code.djangoproject.com/ticket/31317#comment:5>
Django
AlterUniqueTogether crashes
Reporter: Adam (Chainz) | Owner: nobody
Johnson |
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Adam (Chainz) Johnson):
Ah I was not aware the algorithm has been unstable.
--
Ticket URL: <https://code.djangoproject.com/ticket/31317#comment:6>
Django
AlterUniqueTogether crashes
Reporter: Adam Johnson | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: dev
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Ülgen Sarıkavak):
* cc: Ülgen Sarıkavak (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/31317#comment:7>