Opened 9 months ago

Closed 9 months ago

Last modified 8 months ago

#35271 closed Bug (invalid)

Old migrations with UniqueConstraint fail when using psycopg3

Reported by: Adam Zahradník Owned by: nobody
Component: Migrations Version: 5.0
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

We have noticed that migrations generated with Django 4.0.6 containing UniqueConstraint fail to apply under Django 5.0.3 when using psycopg3.

The problematic operation:

migrations.AddConstraint(
    model_name="model",
    constraint=models.UniqueConstraint(
       ["some", "fields", "here"],
       name="constraint_name",
    ),
),

Which fails with the following error:

psycopg.errors.UndefinedObject: data type unknown has no default operator class for access method "btree"
HINT:  You must specify an operator class for the index or define a default operator class for the data type.

However, when using psycopg2, the migration gets applied without any problem.

We also noticed that a migration generated by Django 4.1.5 produces the following operation, which runs successfully on both psycopg versions:

migrations.AddConstraint(
    model_name="model",
    constraint=models.UniqueConstraint(
        models.F("some"),
        models.F("fields"),
        models.F("here"),
        name="constraint_name",
    ),
),

A current way to fix failing migrations is to pass the list of fields to a keyword argument fields:

migrations.AddConstraint(
    model_name="model",
    constraint=models.UniqueConstraint(
       fields=["some", "fields", "here"],
       name="constraint_name",
    ),
),

Change History (7)

comment:1 by Adam Zahradník, 9 months ago

Type: UncategorizedBug

comment:2 by David Sanders, 9 months ago

Triage Stage: UnreviewedAccepted

Thanks for the report 🏆

Looks like it's not just old migrations but a model with the following will produce the same migration & error with psycopg (3)

class Foo(models.Model):
    name = models.CharField()
    label = models.CharField()

    class Meta:
        constraints = [
            models.UniqueConstraint(
                ["name", "label"],
                name="unique_pair",
            ),
        ]

Migration generated:

class Migration(migrations.Migration):
    initial = True

    dependencies = []

    operations = [
        migrations.CreateModel(
            name="Foo",
            fields=[
                (
                    "id",
                    models.BigAutoField(
                        auto_created=True,
                        primary_key=True,
                        serialize=False,
                        verbose_name="ID",
                    ),
                ),
                ("name", models.CharField()),
                ("label", models.CharField()),
            ],
            options={
                "constraints": [
                    models.UniqueConstraint(["name", "label"], name="unique_pair")
                ],
            },
        ),
    ]

comment:3 by David Sanders, 9 months ago

Resolution: invalid
Status: newclosed
Triage Stage: AcceptedUnreviewed

Actually, on second thoughts:

The problem here is the misunderstanding of using UniqueConstraint(['field_1', 'field_2']) - on psycopg2 this is interpreted as the literal ARRAY['field_1'::text, 'field_2'::text']. Which will fail for any value inserted into the table.

Given this, I think this should be categorised as "Invalid" 👍

comment:4 by Adam Zahradník, 9 months ago

When looking at our UniqueConstraint in the model, we never used UniqueConstraint(["field"],...), but either UniqueConstraint(fields=[..]) or UniqueConstraint("field",...). Both of which are documented in the docs: https://docs.djangoproject.com/en/5.0/ref/models/constraints/#uniqueconstraint

I see how UniqueConstraint([...]) would create a wrong constraint, as the array would be accepted as an expression, but I don't think that is the problem here.

We already used this model in production for quite some time, and it is definitely possible to insert values into the table.

comment:5 by David Sanders, 9 months ago

Please paste the _actual_ model field & generated migration for that field so we can determine _exactly_ what is happening.

I'm using Django 4.0.6 and the model

class Foo(models.Model):
    name = models.CharField(max_length=255)
    label = models.CharField(max_length=255)

    class Meta:
        constraints = [
            models.UniqueConstraint(
                "name",
                "label",
                name="unique_pair",
            ),
        ]

generates the migration

        migrations.AddConstraint(
            model_name='foo',
            constraint=models.UniqueConstraint(django.db.models.expressions.F('name'), django.db.models.expressions.F('label'), name='unique_pair'),
        ),

comment:6 by Adam Zahradník, 9 months ago

Upon closer inspection and trying to reproduce this, you are right. The problem was inside the model definition, which was later fixed in our codebase, but the broken migration was still there, but was working on psycopg2 (even though incorrectly), so nobody noticed until now.
`
So, this is indeed not a bug with Django. It can be considered whether passing an iterable to UniqueConstraint should not produce some warning.

comment:7 by David Sanders, 9 months ago

ok thanks for checking 👍

Iterables are valid expressions so I doubt that would be something that would be the correct approach.

If anything, the index should probably coerce basic types like list into their Value(…) counterparts as this solves the issue and is something that is done for db_default already to solve precisely this issue. To explain: psycopg (3) changed the way lists are rendered as SQL from ARRAY['foo', 'bar'] to simply {"foo", "bar"}. The issue is the ambiguous string which requires casting; and Value() expressions are cast.

Last edited 8 months ago by David Sanders (previous) (diff)
Note: See TracTickets for help on using tickets.
Back to Top