Opened 11 years ago
Closed 11 years ago
#22275 closed Bug (fixed)
unique_together with foreign keys fails migration
Reported by: | Ryan Hiebert | Owned by: | Anton Baklanov |
---|---|---|---|
Component: | Migrations | Version: | dev |
Severity: | Release blocker | Keywords: | |
Cc: | antonbaklanov@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
When unique_together
includes foreign keys, sometimes those fields are added in separate migrations. (why?) However, unique_together
remains in the initial migration, before the field referenced in unique_together
is created. Adding the unique_together
constraint then fails.
Attachments (1)
Change History (7)
by , 11 years ago
Attachment: | dj_unique_together.sh added |
---|
comment:1 by , 11 years ago
I also have a repository on Github that has a demonstration of the problem: https://github.com/ryanhiebert/unique_together_dj_migrations. The master
branch demonstrates the problem. The syncdb
branch shows that forcing syncdb
behavior makes it work, and the late-unique
branch demonstrates that waiting until after creating the initial migration to add the unique_together
constraint is a work-around.
comment:2 by , 11 years ago
Severity: | Normal → Release blocker |
---|---|
Triage Stage: | Unreviewed → Accepted |
Type: | Uncategorized → Bug |
Hi,
The provided models work on 1.6 (with syncdb) but indeed they fail on master
(makemigrations
work but migrate
fails with the following traceback:
Traceback (most recent call last): File "manage.py", line 10, in <module> execute_from_command_line(sys.argv) File "./django/core/management/__init__.py", line 427, in execute_from_command_line utility.execute() File "./django/core/management/__init__.py", line 419, in execute self.fetch_command(subcommand).run_from_argv(self.argv) File "./django/core/management/base.py", line 288, in run_from_argv self.execute(*args, **options.__dict__) File "./django/core/management/base.py", line 337, in execute output = self.handle(*args, **options) File "./django/core/management/commands/migrate.py", line 145, in handle executor.migrate(targets, plan, fake=options.get("fake", False)) File "./django/db/migrations/executor.py", line 60, in migrate self.apply_migration(migration, fake=fake) File "./django/db/migrations/executor.py", line 94, in apply_migration migration.apply(project_state, schema_editor) File "./django/db/migrations/migration.py", line 97, in apply operation.database_forwards(self.app_label, schema_editor, project_state, new_state) File "./django/db/migrations/operations/models.py", line 28, in database_forwards schema_editor.create_model(model) File "./django/db/backends/schema.py", line 244, in create_model columns = [model._meta.get_field_by_name(field)[0].column for field in fields] File "./django/db/backends/schema.py", line 244, in <listcomp> columns = [model._meta.get_field_by_name(field)[0].column for field in fields] File "./django/db/models/options.py", line 419, in get_field_by_name % (self.object_name, name)) django.db.models.fields.FieldDoesNotExist: Rabbit has no field named 'parent'
I'm bumping the severity to release blocker
since it's a regression.
Thanks for catching this.
comment:3 by , 11 years ago
Cc: | added |
---|---|
Owner: | changed from | to
Status: | new → assigned |
Similar issue. I'm working on test+patch here now.
comment:4 by , 11 years ago
Has patch: | set |
---|---|
Patch needs improvement: | set |
I'm not opening a pull request for this ticket now since I would like to spend some extra time here.
- This ticket needs more robust test cases
- Currently patch moves all unique_together operations to second migration without making any differences whether all required fields have been created already or not. I decided to do it in this way since
_detect_changes
is a bit over complicated (in my opinion) to handle detection if particular unique_together good enough to be in first migration or it should wait for the next one, as it is done for foreign keys.
I will look into _detect_changes
's code tomorrow one more time to see if it's possible to 'fix' this. Though I'm not completely sure this behavior needs any fixing.
comment:5 by , 11 years ago
It looks good enough to me; the optimisation of where to put it can be addressed along with the same optimisation for ForeignKeys (it currently errs a bit on the "splitting up" side), but in the interest of getting the beta out I'm committing it right now, as it fixes the bug.
comment:6 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Shell script that demonstrates unique_together issue