Opened 5 years ago

Closed 5 years ago

#30172 closed Bug (fixed)

Incorrect migration applying for new Meta.constraints/indexes and field check/unique or unique/index_together constraints with same fields for postgres

Reported by: Pavel Tyslacki Owned by: Pavel Tyslacki
Component: Migrations Version: 2.2
Severity: Release blocker Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Case 1: trying remove check constraint from field when CheckConstraint exists in Meta.constraints with same field will remove both constraints:

class TestCheck(models.Model):
    f1 = models.PositiveIntegerField()  # will change to `f1 = models.IntegerField()`

    class Meta:
        constraints = [
            models.CheckConstraint(check=models.Q(f1__gte=0), name='TestCheck_f1_check'),
        ]


# initial migration:

        migrations.CreateModel(
            name='TestCheck',
            fields=[
                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
                ('f1', models.PositiveIntegerField()),
            ],
        ),
        migrations.AddConstraint(
            model_name='testcheck',
            constraint=models.CheckConstraint(check=models.Q(f1__gte=0), name='TestCheck_f1_check'),
        ),

# applied migration:

        migrations.AlterField(
            model_name='testcheck',
            name='f1',
            field=models.IntegerField(),
        ),

Case 2: trying remove unique=True from field when UniqueConstraint exists in Meta.constraints with same field will remove both constraints or raise exception if UniqueConstraint has condition:

class TestUniqCondField(models.Model):
    f1 = models.IntegerField(unique=True)  # will change to `f1 = models.IntegerField()`

    class Meta:
        constraints = [
            models.UniqueConstraint(fields=['f1'], name='TestUniqCondField_f1_uniq', condition=models.Q(f1__gte=0)),
        ]


# initial migration:

        migrations.CreateModel(
            name='TestUniqCondField',
            fields=[
                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
                ('f1', models.IntegerField(unique=True)),
            ],
        ),
        migrations.AddConstraint(
            model_name='testuniqcondfield',
            constraint=models.UniqueConstraint(condition=models.Q(f1__gte=0), fields=('f1',), name='TestUniqCondField_f1_uniq'),
        ),


# applied migration:

        migrations.AlterField(
            model_name='testuniqcondfield',
            name='f1',
            field=models.IntegerField(),
        ),

Case 3: trying remove Meta.unique_together for same UniqueConstraint.fields in Meta.constraints get incorrect state via introspection (the same if UniqueConstraint has condition):

class TestUniqTogether(models.Model):
    f1 = models.IntegerField()
    f2 = models.IntegerField()

    class Meta:
        unique_together = [
            ('f1', 'f2'),  # will be removed
        ]

        constraints = [
            models.UniqueConstraint(fields=['f1', 'f2'], name='TestUniqTogether_f1_f2_uniq'),
        ]


# initial migration:

        migrations.CreateModel(
            name='TestUniqTogether',
            fields=[
                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
                ('f1', models.IntegerField()),
                ('f2', models.IntegerField()),
            ],
        ),
        migrations.AddConstraint(
            model_name='testuniqtogether',
            constraint=models.UniqueConstraint(fields=('f1', 'f2'), name='TestUniqTogether_f1_f2_uniq'),
        ),
        migrations.AlterUniqueTogether(
            name='testuniqtogether',
            unique_together={('f1', 'f2')},
        ),


# applied migration

        migrations.AlterUniqueTogether(
            name='testuniqtogether',
            unique_together=set(),
        ),

Case 4: trying remove Meta.index_together for same Index.fields in Meta.indexes get incorrect state via introspection (the same if Index has condition):

class TestIndexTogether(models.Model):
    f1 = models.IntegerField()
    f2 = models.IntegerField()

    class Meta:
        index_together = [
            ('f1', 'f2'),  # will be removed
        ]

        indexes = [
            models.Index(fields=['f1', 'f2'], name='TestIndexTogether_f1_f2_index'),
        ]


# initial migration:

        migrations.CreateModel(
            name='TestIndexTogether',
            fields=[
                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
                ('f1', models.IntegerField()),
                ('f2', models.IntegerField()),
            ],
        ),
        migrations.AddIndex(
            model_name='testindextogether',
            index=models.Index(fields=['f1', 'f2'], name='TestIndexTogether_f1_f2_index'),
        ),
        migrations.AlterIndexTogether(
            name='testindextogether',
            index_together={('f1', 'f2')},
        ),


# applied migration:

        migrations.AlterIndexTogether(
            name='testindextogether',
            index_together=set(),
        ),

Change History (11)

comment:1 by Pavel Tyslacki, 5 years ago

Summary: Incorrect migration applying for new Meta.constraints/indexes and field check/unique or unique/index_together with same fieldsIncorrect migration applying for new Meta.constraints/indexes and field check/unique or unique/index_together constraints with same fields for postgres

comment:2 by Pavel Tyslacki, 5 years ago

Has patch: set
Owner: changed from nobody to Pavel Tyslacki
Status: newassigned
Last edited 5 years ago by Tim Graham (previous) (diff)

comment:3 by Tim Graham, 5 years ago

Patch needs improvement: set
Triage Stage: UnreviewedAccepted

Can you look into the SQLite failures?

in reply to:  3 comment:4 by Pavel Tyslacki, 5 years ago

Replying to Tim Graham:

Can you look into the SQLite failures?

Yep. Hope will fix soon.

comment:5 by Asif Saifuddin Auvi, 5 years ago

Patch needs improvement: unset

comment:6 by Tim Graham, 5 years ago

Severity: NormalRelease blocker

Marking as a release blocker since it's a bug in a new feature (Meta.constraints).

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

In 4bb859e:

Refs #30172 -- Prevented removing a field's check or unique constraint from removing Meta constraints.

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

In 5c17c273:

Refs #30172 -- Prevented removing a model Meta's index/unique_together from removing Meta constraints/indexes.

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

In 3dd5e717:

[2.2.x] Refs #30172 -- Prevented removing a field's check or unique constraint from removing Meta constraints.

Backport of 4bb859e24694f6cb8974ed9d2225f18214338ea3 from master.

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

In 2a92e2e:

[2.2.x] Refs #30172 -- Prevented removing a model Meta's index/unique_together from removing Meta constraints/indexes.

Backport of 5c17c273ae2d7274f1fa78218b3b74690efddb86 from master.

comment:11 by Tim Graham, 5 years ago

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.
Back to Top