Opened 12 months ago

Closed 12 months ago

Last modified 12 months ago

#23288 closed Bug (fixed)

Unable to correctly access data in table during migration

Reported by: mtthw-meyer Owned by: andrewgodwin
Component: Migrations Version: 1.7-rc-2
Severity: Release blocker Keywords:
Cc: info@…, cmawebsite@… 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 created a profile and then setup some relations to it. When I change the relation from the profile to the User model and try to access the data after the models.py file has been changed but before the migration has been completed the members field returns only a single profile objects instead of the ones that were assigned. Dumping the data shows that users with id 2,3,4,5 are in team with id 1, but when accessing the field from the migration only user id 2 is returned.

Attachments (1)

migration_example.tgz (14.2 KB) - added by mtthw-meyer 12 months ago.
Example application that recreates the problem

Download all attachments as: .zip

Change History (12)

Changed 12 months ago by mtthw-meyer

Example application that recreates the problem

comment:1 Changed 12 months ago by collinanderson

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Severity changed from Normal to Release blocker
  • Triage Stage changed from Unreviewed to Accepted

Yikes! I regret to say I can reproduce this. And it happens even without the Profile-User change to your models.py.

comment:2 Changed 12 months ago by collinanderson

I'm out of time, but I put the example on github if someone else wants to give it a try.

compare running the this code by hand in the shell vs in the migration

https://github.com/collinanderson/ticket23288/blob/d3354319e78cf46077fc46e61ff70e201704b264/example/migrations/0002_auto_20140813_1645.py#L9

comment:3 Changed 12 months ago by Markush2010

  • Cc info@… added
  • Needs tests set

I can confirm the issue too. The repository holds a simplified test case: https://github.com/collinanderson/ticket23288

comment:4 Changed 12 months ago by collinanderson

  • Cc cmawebsite@… added

The issue happens when querying a many-to-many. The ORM is incorrectly using the ForeignKey relationship.
A deep link to the test case: https://github.com/collinanderson/ticket23288/commit/21a501c963f8a2c50278d438ef1b87cc7cd3031d#diff-186cb93781c2f86fd8d75aac5cd19c2dR6

Last edited 12 months ago by collinanderson (previous) (diff)

comment:5 Changed 12 months ago by andrewgodwin

  • Owner changed from nobody to andrewgodwin
  • Status changed from new to assigned

Looking at this now.

comment:6 Changed 12 months ago by andrewgodwin

Well that was nasty. The root cause was that deconstruct() wasn't including related_name, and so both the FK and the M2M ended up with the same related_name - and without model validation running against the migration models (as they're supposedly already validated if deconstruct is accurate) Django didn't catch this and so the filter it passed to the query backend was team__id = X, and Django picked the first thing it knew about with a related_name of "team" - the ForeignKey, not the ManyToMany.

The fix makes sure deconstruct also knows about related_name and a bunch of other similar fields that were missing before (most notably, I have left out limit_choices_to for now as it has no database or model layer effect and returns a set of mostly unserialisable things).

If you have existing migrations from before the fix, you'll either need to regenerate them or manually add related_name into the field definitions yourself (doing this makes the supplied test case pass for me).

comment:7 Changed 12 months ago by Andrew Godwin <andrew@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 4d2f9c38e835383fe42548353890ed4a302eaa58:

Fixed #23288: deconstruct() ignoring related_name

comment:8 Changed 12 months ago by collinanderson

  • Has patch set
  • Needs tests unset
  • Resolution fixed deleted
  • Status changed from closed to new
  • Triage Stage changed from Accepted to Ready for checkin

Could use a [1.7.x] backport.

comment:9 Changed 12 months ago by collinanderson

  • Resolution set to fixed
  • Status changed from new to closed

sorry nevermind. it's right here, the trac hook must have not worked: https://github.com/django/django/commit/b5784048e085e8db5d0e959c12278ef727994e98

comment:10 Changed 12 months ago by Tim Graham <timograham@…>

In 1d79d08d9aef1565cf56d50f0742285efae53631:

Fixed #23294 -- Add related_name to existing migrations.

Thanks to Florian Apolloner for the review; refs #23288.

comment:11 Changed 12 months ago by Tim Graham <timograham@…>

In 41cf159c0ac0a7380874cb6f17464dd741ad1d9b:

[1.7.x] Fixed #23294 -- Add related_name to existing migrations.

Thanks to Florian Apolloner for the review; refs #23288.

Backport of 1d79d08d9a from master

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