[Django] #23452: Infinite migrations with empty unique_together
Django
-------------------------------+--------------------
Reporter: fwkroon | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: 1.7
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
Creating a model with an empty unique_together causes makemigrations to
continuously generate migrations.
To reproduce, create a model with an empty unique_together:
{{{#!python
from django.db import models
class EmptyUniqueTogether(models.Model):
class Meta:
unique_together = ()
name = models.CharField(max_length=256, db_index=True)
}}}
python manage.py makemigrations
{{{
Migrations for 'uniquetest':
0001_initial.py:
- Create model EmptyUniqueTogether
}}}
Immediately running makemigrations again results in an infinite series of
migrations:
{{{
$ python manage.py makemigrations
Migrations for 'uniquetest':
0002_auto_20140908_1923.py:
- Alter unique_together for emptyuniquetogether (0 constraint(s))
$ python manage.py makemigrations
Migrations for 'uniquetest':
0003_auto_20140908_1923.py:
- Alter unique_together for emptyuniquetogether (0 constraint(s))
}}}
The generated migrations all look like the following:
{{{#!python
# -*- coding: utf-8 -*-
from __future__ import unicode_literals
from django.db import models, migrations
class Migration(migrations.Migration):
dependencies = [
('uniquetest', '0001_initial'),
]
operations = [
migrations.AlterUniqueTogether(
name='emptyuniquetogether',
unique_together=set([]),
),
]
}}}
Tested with both python 2.7 and python 3.3, on Django 1.7 as well as the
development trunk. All combinations behaved identically.
I think an empty unique_together is probably not meaningful, so perhaps an
exception should be raised instead?
--
Ticket URL: <https://code.djangoproject.com/ticket/23452>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: fwkroon | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: 1.7
Severity: Normal | Resolution:
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 timgraham):
* needs_docs: => 0
* needs_better_patch: => 0
* type: Uncategorized => Bug
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
I don't think it needs to raise an error, but it's probably not common so
not marking as a release blocker. I'd want to backport the fix to 1.7 when
we have a patch though.
--
Ticket URL: <https://code.djangoproject.com/ticket/23452#comment:1>
Django
Reporter: fwkroon | Owner: Markush2010
Type: Bug | Status: assigned
Component: Migrations | Version: 1.7
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Markush2010):
* cc: info+coding@… (added)
* status: new => assigned
* owner: nobody => Markush2010
Comment:
First of all, this also affects the `index_together` option.
I'm working on a pull-request. Not sure though, which way I should go.
Here's what's happening:
The `CreateModel` doesn't include those two options at all but adds them,
if they evaluate to `True` (`if model_state.options.get('unique_together',
None)`) as separate operations. Thus an empty `unique_together`
declaration is never added initially. The `AlterUniqueTogether` or
`AlterIndexTogether` on the other side are added if the given value (`()`
is different to `None`).
We now have two options:
1. Always add an `AlterUniqueTogether` or `AlterIndexTogether` operation,
even if it's empty (`()`).
2. Never add an `AlterUniqueTogether` or `AlterIndexTogether` operation if
it's empty (`()`).
I'd go with 2.
--
Ticket URL: <https://code.djangoproject.com/ticket/23452#comment:2>
Django
Reporter: fwkroon | Owner: Markush2010
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Markush2010):
* has_patch: 0 => 1
Comment:
I opened a pull request: https://github.com/django/django/pull/3207
--
Ticket URL: <https://code.djangoproject.com/ticket/23452#comment:3>
Django
Reporter: fwkroon | Owner: Markush2010
Component: Migrations | Version: 1.7
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"6d5958c7a358d8ad0037fdd4922a65e19d12d77c"]:
{{{
#!CommitTicketReference repository=""
revision="6d5958c7a358d8ad0037fdd4922a65e19d12d77c"
Fixed #23452 -- Prevented infinite migrations for empty
unique/index_together.
Thanks fwkroon for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23452#comment:4>
Django
Reporter: fwkroon | Owner: Markush2010
Type: Bug | Status: closed
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"67872bfff12de7dcf22c966970fe21f65fe593c8"]:
{{{
#!CommitTicketReference repository=""
revision="67872bfff12de7dcf22c966970fe21f65fe593c8"
[1.7.x] Fixed #23452 -- Prevented infinite migrations for empty
unique/index_together.
Thanks fwkroon for the report.
Backport of 6d5958c7a3 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23452#comment:5>