Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#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 Changed 5 years ago by Tim Graham

Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

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.

comment:2 Changed 5 years ago by Markus Holtermann

Cc: info+coding@… added
Owner: changed from nobody to Markus Holtermann
Status: newassigned

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.

comment:3 Changed 5 years ago by Markus Holtermann

Has patch: set

comment:4 Changed 5 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In 6d5958c7a358d8ad0037fdd4922a65e19d12d77c:

Fixed #23452 -- Prevented infinite migrations for empty unique/index_together.

Thanks fwkroon for the report.

comment:5 Changed 5 years ago by Tim Graham <timograham@…>

In 67872bfff12de7dcf22c966970fe21f65fe593c8:

[1.7.x] Fixed #23452 -- Prevented infinite migrations for empty unique/index_together.

Thanks fwkroon for the report.

Backport of 6d5958c7a3 from master

Note: See TracTickets for help on using tickets.
Back to Top