Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#23906 closed Bug (invalid)

"ValueError: Found wrong number (0) of constraints" when migrating backwards with from alter_unique_together migration.

Reported by: liavkoren Owned by: nobody
Component: Migrations Version: 1.7
Severity: Normal Keywords: alter_unique_together migrations constraints valueError
Cc: info+coding@… 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 a database that was created with django 1.6 and south. We've subsequently upgraded to 1.7.0, removed our south migrations and created django-migrations.

One of our models contains a unique_together constraint that includes four of its attributes. When I attempt to backwards migrate the app containing this model I get "ValueError: Found wrong number (0) of constraints for foo_model(foo, bar, baz, quux)".

I haven't been able to replicate this error by creating a test model -- it seems possible that this may be related to models created in django 1.6/south and then migrated backwards in 1.7.

The problem seems to be in django.db.backends.schema._contraint_names, which is supposed to return a list of relevant constrains for a set of columns.

In order to do this, it is comparing an input set of columns against the result returned from a call to
BaseDatabaseSchemaEditor.connection.introspection.get_constraints and filtering the results. The first test in the filter loop is:

    if column_names is None or column_names == infodict['columns']:

which is failing because it involves a comparison of lists which are in different orders:

(Pdb++) column_names
[u'foo', u'bar', u'baz', u'quux']
(Pdb++) infodict['columns']
[u'baz', u'bar', u'quux', u'foo']
(Pdb++) infodict['columns'] == column_names
False
(Pdb++) sorted(infodict['columns']) == sorted(column_names)
True

I have been able to fix this and migrate backwards by changing the django.db.backends.schema.BaseDatabaseSchemaEditor._constraint_names method to compare sets rather than lists:

    def _constraint_names(self, model, column_names=None, unique=None, primary_key=None, index=None, foreign_key=None, check=None):
        """
        Returns all constraint names matching the columns and conditions
        """
        # column_names = list(column_names) if column_names else None
        column_names = set(column_names) if column_names else None
        with self.connection.cursor() as cursor:
            constraints = self.connection.introspection.get_constraints(cursor, model._meta.db_table)
        result = []
        for name, infodict in constraints.items():
            # if column_names is None or column_names == infodict['columns']:
            if column_names is None or column_names == set(infodict['columns']):
                if unique is not None and infodict['unique'] != unique:
                    continue
                if primary_key is not None and infodict['primary_key'] != primary_key:
                    continue
                if index is not None and infodict['index'] != index:
                    continue
                if check is not None and infodict['check'] != check:
                    continue
                if foreign_key is not None and not infodict['foreign_key']:
                    continue
                result.append(name)
        return result

I'm happy to submit a PR and tests for this issue, but I expect it's going to be non-trivial to reproduce.

Change History (3)

comment:1 by Carl Meyer, 9 years ago

I would guess that reproducing for purposes of writing a tests shouldn't be too hard with some raw SQL; it seems this is simply a matter of the order in which columns are listed in a constraint. The question is whether the suggested fix is actually correct. Does it break any existing tests?

How does migrations currently handle it if you change the ordering of a unique_together without otherwise changing the constituent fields?

I notice there is an ORDER BY ordinal_position in at least the PostgreSQL implementation of get_constraints, suggesting that the order of fields in a constraint is relevant.

I think that's the basic question here: either a constraint with the same fields in a different order is semantically the same constraint (in which case your suggested fix seems right), or its semantically a different constraint (in which case the current behavior is correct, and you just have a mismatch between the state of your database and what migrations thinks is the state of your database, which is something you need to deal with locally).

I don't actually know which of those is correct in terms of the underlying database semantics, though the fact that at least PG clearly tracks field ordering within a constraint suggests the latter.

comment:2 by Markus Holtermann, 9 years ago

Cc: info+coding@… added
Resolution: invalid
Status: newclosed

As far as I know the order of the items in the too_together lists does matter, at least on MySQL a unique constraint on a,b is different to b,a (or at least was for a very long time). That being said, your "fix" is wrong and, as Carl already said, your database just doesn't have the same state as your models represent.

comment:3 by Ramiro Morales, 9 years ago

FYI,

TL;DR: Review your schema as is exists before you start using Django migrations. Chances are that the order of fields in the unique_together constraint there doesn't match what the models.py says, and hasn't for a long time.

We found the same problem with a similar setup (Django 1.6 + South migrations being ported to Django 1.7) and scenario.

What happened in our case is that the history of the model involved is like this:

1) Model got created without unique_together constraint
2) A unique_together = ('char_field', 'fk_field') got added at some point. South migtation syntax:

db.create_unique(u'app_modelname', ['char_field', 'fk_field_id'])

What happened is that starting from that point it seems South recorded fields of the unique_together in the wrong order because all the frozen representations of the model in every migration file created since then contained the following excerpt:

'Meta': {'unique_together': "(('fk_field', 'str_field'),)", 'object_name': 'ModelName'},

So, when it came time to create a initial migration using Django 1.7 native migrations framework, it picked the original ('char_field', 'fk_field') read from the models.py file.

But when:

  • No DB schema is created by Django 1.7 because it already exists and we let migrate fake the initial Django migrations (or we pass --fake to it) when migrating forward.
  • We try to migrate back to migration zero (i.e. the state where the South->Djnago 1.7 handover happened), Django detects the order of the constraint as it exists in the DB doesn't match what it should be (i.e. what models.py says)

and then the error message reported by the OP is shown.

Last edited 9 years ago by Ramiro Morales (previous) (diff)
Note: See TracTickets for help on using tickets.
Back to Top