#23452 closed Bug (fixed)
Infinite migrations with empty unique_together
Reported by: | fwkroon | Owned by: | Markus Holtermann |
---|---|---|---|
Component: | Migrations | Version: | 1.7 |
Severity: | Normal | Keywords: | |
Cc: | info+coding@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Creating a model with an empty unique_together causes makemigrations to continuously generate migrations.
To reproduce, create a model with an empty unique_together:
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:
# -*- 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?
Change History (5)
comment:1 by , 10 years ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Type: | Uncategorized → Bug |
comment:2 by , 10 years ago
Cc: | added |
---|---|
Owner: | changed from | to
Status: | new → assigned |
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:
- Always add an
AlterUniqueTogether
orAlterIndexTogether
operation, even if it's empty (()
). - Never add an
AlterUniqueTogether
orAlterIndexTogether
operation if it's empty (()
).
I'd go with 2.
comment:3 by , 10 years ago
Has patch: | set |
---|
I opened a pull request: https://github.com/django/django/pull/3207
comment:4 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
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.