Opened 10 years ago

Closed 2 years ago

Last modified 2 years ago

#23740 closed Bug (fixed)

Cannot drop unique_together constraint on a single field with its own unique=True constraint

Reported by: Mihail Milushev Owned by: David Wobrock
Component: Migrations Version: 1.7
Severity: Normal Keywords: unique_together
Cc: David Wobrock Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I have an erroneous unique_together constraint on a model's primary key (unique_together = (('id',),)) that cannot be dropped by a migration. Apparently the migration tries to find all unique constraints on the column and expects there to be only one, but I've got two — the primary key and the unique_together constraint:

Indexes:
    "foo_bar_pkey" PRIMARY KEY, btree (id)
    "foo_bar_id_1c3b3088c74c3b17_uniq" UNIQUE CONSTRAINT, btree (id)

Database is PostgreSQL, if that makes any difference.

Change History (16)

comment:1 by Markus Holtermann, 10 years ago

Can you share a traceback that shows how and where the migration fails, please.

comment:2 by Mihail Milushev, 10 years ago

Here it is:

Traceback (most recent call last):
  File "./bin/django", line 56, in <module>
    sys.exit(djangorecipe.manage.main('project.settings.local.foobar'))
  File "/foo/bar/eggs/djangorecipe-1.10-py2.7.egg/djangorecipe/manage.py", line 9, in main
    management.execute_from_command_line(sys.argv)
  File "/foo/bar/eggs/Django-1.7-py2.7.egg/django/core/management/__init__.py", line 385, in execute_from_command_line
    utility.execute()
  File "/foo/bar/eggs/Django-1.7-py2.7.egg/django/core/management/__init__.py", line 377, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/foo/bar/eggs/Django-1.7-py2.7.egg/django/core/management/base.py", line 288, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/foo/bar/eggs/Django-1.7-py2.7.egg/django/core/management/base.py", line 338, in execute
    output = self.handle(*args, **options)
  File "/foo/bar/eggs/Django-1.7-py2.7.egg/django/core/management/commands/migrate.py", line 160, in handle
    executor.migrate(targets, plan, fake=options.get("fake", False))
  File "/foo/bar/eggs/Django-1.7-py2.7.egg/django/db/migrations/executor.py", line 63, in migrate
    self.apply_migration(migration, fake=fake)
  File "/foo/bar/eggs/Django-1.7-py2.7.egg/django/db/migrations/executor.py", line 97, in apply_migration
    migration.apply(project_state, schema_editor)
  File "/foo/bar/eggs/Django-1.7-py2.7.egg/django/db/migrations/migration.py", line 107, in apply
    operation.database_forwards(self.app_label, schema_editor, project_state, new_state)
  File "/foo/bar/eggs/Django-1.7-py2.7.egg/django/db/migrations/operations/models.py", line 253, in database_forwards
    getattr(new_model._meta, self.option_name, set()),
  File "/foo/bar/eggs/Django-1.7-py2.7.egg/django/db/backends/schema.py", line 315, in alter_unique_together
    ", ".join(columns),
ValueError: Found wrong number (2) of constraints for foo_bar(id)

(some identifier names and paths were changed to protect the innocent :))

comment:3 by Matt Westcott, 10 years ago

Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

Confirmed on current master. On a new postgresql-based project:

  • create a new app 'foo' with the following models.py
from django.db import models

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

    class Meta:
        unique_together = (('id',),)
  • ./manage.py makemigrations
  • ./manage.py migrate
  • comment out the 'class Meta'
  • ./manage.py makemigrations
  • ./manage.py migrate

This is not specific to the primary key field - it happens more generally on single-field unique_together constraints that duplicate a unique=True constraint, such as:

class Bar(models.Model):
    name = models.CharField(max_length=255, unique=True)

    class Meta:
        unique_together = (('name'),)

comment:4 by Matt Westcott, 10 years ago

Keywords: unique_together added

comment:5 by Matt Westcott, 10 years ago

Summary: Cannot drop unique_together constraint on primary keyCannot drop unique_together constraint on a single field with its own unique=True constraint

Can't see a good way of fixing this without breaking backwards compatibility. The alter_unique_together logic in db/backends/schema.py relies on the UNIQUE constraints generated by unique_together to be distinguishable within the database from constraints generated through other mechanisms (e.g. unique=True) - but this isn't possible when the unique_together rule only contains a single field.

Another way this manifests itself is to perform the following steps, in separate migrations:

  • create a model
  • add unique=True to one if its fields
  • add a unique_together constraint for that same field

This results in two identical calls to _create_unique_sql, leading to a 'relation "foo_bar_name_1e64ed8ec8cfa1c7_uniq" already exists' error.

in reply to:  5 comment:6 by Dave Peticolas, 9 years ago

Replying to matthewwestcott:

Can't see a good way of fixing this without breaking backwards compatibility. The alter_unique_together logic in db/backends/schema.py relies on the UNIQUE constraints generated by unique_together to be distinguishable within the database from constraints generated through other mechanisms (e.g. unique=True) - but this isn't possible when the unique_together rule only contains a single field.

I have encountered this same issue. Could you explain the backwards compatibility issue here? It seems to me that the bug is in attempting to drop or create an index when that is not called for, either because the index is required by a different constraint or was already created by another constraint. Is that behavior that needs to be kept.

comment:7 by Adrian Torres, 3 years ago

Just got hit by this bug, I believe that at least in recent versions of Django (at least 3.2.13) it is now possible to distinguish unique=True constraints from unique_together ones.

At least in the application I'm working on, there seem to be two different naming schemes:

  • For UNIQUE CONSTRAINT created with unique=True the naming scheme seems to be <table_name>_<field_name>_key
  • For UNIQUE CONSTRAINT created with unique_together the naming scheme seems to be <table_name>_<field_names>_<hash>_uniq

Note that this is extrapolated from me looking at my application's database schemas, I haven't read the code so I am unsure if this is in fact what happens all of the time.

So in the case where we would want to delete a unique_together constraint that had a single field, we'd just look for foo_bar_%_uniq and drop that constraint, here is an SQL statement (in PostgreSQL) that I used in a manual migration (originally from https://stackoverflow.com/a/12396684) in case it can be useful to someone that comes across this bug again:

            DO
            $body$
            DECLARE
                _con text := (
                    SELECT quote_ident(conname)
                    FROM pg_constraint
                    WHERE conrelid = 'my_table'::regclass
                    AND contype = 'u'
                    AND conname LIKE 'my_table_my_field_%_uniq'
                );
            BEGIN
                EXECUTE
                    'ALTER TABLE my_table DROP CONSTRAINT ' || _con;
            END
            $body$;

So I think it's possible to fix this bug now?

comment:8 by Claude Paroz, 3 years ago

As a workaround, it may be possible to migrate the unique_together to a UniqueConstraint, keeping the same index name, and then dropping the UniqueConstraint. Just an idea, untested.

comment:9 by David Wobrock, 2 years ago

Cc: David Wobrock added
Has patch: set
Owner: changed from nobody to David Wobrock
Status: newassigned

Hey there,

It's indeed still (mostly) relevant.
I've tried to tackle the issue, here is a first draft of the PR.

"Mostly" because I couldn't reproduce the part that Matt is describing about adding a unique_together:

Another way this manifests itself is to perform the following steps, in separate migrations:

  • create a model
  • add unique=True to one if its fields
  • add a unique_together constraint for that same field

This results in two identical calls to _create_unique_sql, leading to a 'relation "foo_bar_name_1e64ed8ec8cfa1c7_uniq" already exists' error.

For the rest, removing the unique_together on a PK field and on a unique=True field should work now, when we base the constraint checking on how the unique_together names are generated by default.
However, this will break dropping such a constraint if it has been renamed manually. I think this should be fine, as the name is generated by Django and never really exposed - so I guess it's okay to regard this name as internals of Django, and we can rely on it.

Feel free to tell me what you think of the PR :)

comment:10 by Mariusz Felisiak, 2 years ago

Patch needs improvement: set

comment:11 by David Wobrock, 2 years ago

Patch needs improvement: unset

comment:12 by Mariusz Felisiak, 2 years ago

Triage Stage: AcceptedReady for checkin

comment:13 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

In 115a978:

Refs #23740 -- Added BaseDatabaseSchemaEditor._unique_constraint_name().

comment:14 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

Resolution: fixed
Status: assignedclosed

In b949e40e:

Fixed #23740 -- Fixed removing unique_together constraint if exists primary key/unique constraint on the same field.

comment:15 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

In 176d5ec2:

[4.1.x] Refs #23740 -- Added BaseDatabaseSchemaEditor._unique_constraint_name().

Backport of 115a978fceac4c4de2f9fc70ce19001c3f6d6918 from main

comment:16 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

In ae7aecc9:

[4.1.x] Fixed #23740 -- Fixed removing unique_together constraint if exists primary key/unique constraint on the same field.

Backport of b949e40e8c32a393f480399e2d70b653108098b6 from main

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