#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 , 10 years ago
comment:2 by , 10 years ago
Cc: | added |
---|---|
Resolution: | → invalid |
Status: | new → closed |
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 , 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.
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 ofget_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.