[Django] #22275: unique_together with foreign keys fails migration
Django
-------------------------------+--------------------
Reporter: ryanhiebert | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/22275>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: ryanhiebert | Owner: nobody
Type: Uncategorized | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by ryanhiebert):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/22275#comment:1>
Django
Reporter: ryanhiebert | Owner: nobody
Type: Bug | Status: new
Component: Migrations | Version: master
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by bmispelon):
* type: Uncategorized => Bug
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/22275#comment:2>
Django
Reporter: ryanhiebert | Owner: bak1an
Type: Bug | Status: assigned
Component: Migrations | Version: master
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by bak1an):
* status: new => assigned
* cc: antonbaklanov@… (added)
* owner: nobody => bak1an
Comment:
[https://code.djangoproject.com/ticket/22035 Similar issue.] I'm working
on test+patch here now.
--
Ticket URL: <https://code.djangoproject.com/ticket/22275#comment:3>
Django
Reporter: ryanhiebert | Owner: bak1an
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by bak1an):
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
Comment:
[https://github.com/bak1an/django/compare/django:master...bak1an:ticket_22275
basic test + naive patch]
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/22275#comment:4>
Django
Reporter: ryanhiebert | Owner: bak1an
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by andrewgodwin):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/22275#comment:5>
Django
Reporter: ryanhiebert | Owner: bak1an
Component: Migrations | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by Andrew Godwin <andrew@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"81f5408c7a16b8c79053950f05fe7a873506ca55"]:
{{{
#!CommitTicketReference repository=""
revision="81f5408c7a16b8c79053950f05fe7a873506ca55"
Fixed #22275: unique_together broken if ForeignKey split into new file.
Thanks to bak1an for the patch.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22275#comment:6>