[Django] #22508: select_related on a foreign related object fails to load fields on original object
Django
object
----------------------------------------------+--------------------
Reporter: boxm | Owner: nobody
Type: Uncategorized | Status: new
Component: Database layer (models, ORM) | Version: 1.6
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
Consider these models:
{{{
class Show(models.Model):
pass
class Poll(models.Model):
event = models.ForeignKey('Event')
class Event(models.Model)
show = models.ForeignKey(Show)
}}}
The following code fails to load the poll.event.show attribute:
{{{
In [3]: event = Event.objects.all()[2]
In [4]: poll = event.poll_set.select_related('event',
'event__show').all()[0]
In [5]: hasattr(poll.event, '_show_cache')
Out[5]: False
}}}
For comparison, in 1.4 the same code produces:
{{{
In [12]: event = Event.objects.all()[2]
In [13]: poll = event.poll_set.select_related('event',
'event__show').all()[0]
In [14]: hasattr(poll.event, '_show_cache')
Out[14]: True
}}}
The bug appears to be in the known_objects code around line 247 in
db/models/query.py which looks like it will overwrite the poll.event
attribute that was just retrieved from the DB (via select_related) and
replaces it with the parent model, which in this case doesn't have the
show attribute loaded.
Since the ORM was explicitly instructed to load the Event model in the
call, it shouldn't then replace the poll.event attribute with the
previously-known parent model.
--
Ticket URL: <https://code.djangoproject.com/ticket/22508>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
object
Reporter: boxm | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.6
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by boxm):
* needs_better_patch: => 0
* type: Uncategorized => Bug
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/22508#comment:1>
Django
object
Reporter: boxm | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.6
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timo):
Bisected to 1e6c3368f2517f32b0651d68277ea8c9ef81d9b2
--
Ticket URL: <https://code.djangoproject.com/ticket/22508#comment:2>
Django
object
Reporter: boxm | Owner: aaugustin
Type: Bug | Status: assigned
Component: Database layer | Version: 1.6
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* owner: nobody => aaugustin
* status: new => assigned
Comment:
I knew I shouldn't have touched this code...
--
Ticket URL: <https://code.djangoproject.com/ticket/22508#comment:3>
Django
object
Reporter: boxm | Owner: aaugustin
Type: Bug | Status: assigned
Component: Database layer | Version: 1.6
(models, ORM) | Resolution:
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by timo):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/22508#comment:4>
Django
object
Reporter: boxm | Owner: aaugustin
Type: Bug | Status: assigned
Component: Database layer | Version: 1.6
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
Comment:
I've sent a pull request: https://github.com/django/django/pull/2646. The
fix is straightforward.
Since this can cause silent performance regressions, I would suggest
backporting f574220f0988f3aa1aca4f133887fbde0e5a6f10 to 1.7, 1.6 and 1.5.
`QuerySet.iterator` is growing a bit complex and might benefit from a
refactoring.
--
Ticket URL: <https://code.djangoproject.com/ticket/22508#comment:5>
Django
object
Reporter: boxm | Owner: aaugustin
Type: Bug | Status: assigned
Component: Database layer | Version: 1.6
(models, ORM) | Resolution:
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* stage: Accepted => Ready for checkin
Comment:
Looks good, also for backporting, with a slight reserve about 1.5 being in
security/data-loss bug fix only.
--
Ticket URL: <https://code.djangoproject.com/ticket/22508#comment:6>
Django
object
Reporter: boxm | Owner: aaugustin
Component: Database layer | Version: 1.6
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Aymeric Augustin <aymeric.augustin@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"f574220f0988f3aa1aca4f133887fbde0e5a6f10"]:
{{{
#!CommitTicketReference repository=""
revision="f574220f0988f3aa1aca4f133887fbde0e5a6f10"
Fixed #22508 -- Avoided overwriting select_related.
Previously, known related objects overwrote related objects loaded
though select_related. This could cancel the effect of select_related
when it was used over more than one level.
Thanks boxm for the bug report and timo for bisecting the regression.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22508#comment:7>
Django
object
Reporter: boxm | Owner: aaugustin
Type: Bug | Status: closed
Component: Database layer | Version: 1.6
(models, ORM) | Resolution: fixed
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Aymeric Augustin <aymeric.augustin@…>):
In [changeset:"e9d0ef19bc87aafd71d513cd927db2655267809b"]:
{{{
#!CommitTicketReference repository=""
revision="e9d0ef19bc87aafd71d513cd927db2655267809b"
[1.7.x] Fixed #22508 -- Avoided overwriting select_related.
Previously, known related objects overwrote related objects loaded
though select_related. This could cancel the effect of select_related
when it was used over more than one level.
Thanks boxm for the bug report and timo for bisecting the regression.
Backport of f574220f from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22508#comment:8>
Django
object
Reporter: boxm | Owner: aaugustin
Type: Bug | Status: closed
Component: Database layer | Version: 1.6
(models, ORM) | Resolution: fixed
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Aymeric Augustin <aymeric.augustin@…>):
In [changeset:"b6d3212190b114f1ef90d7ee9edcdae046432779"]:
{{{
#!CommitTicketReference repository=""
revision="b6d3212190b114f1ef90d7ee9edcdae046432779"
[1.6.x] Fixed #22508 -- Avoided overwriting select_related.
Previously, known related objects overwrote related objects loaded
though select_related. This could cancel the effect of select_related
when it was used over more than one level.
Thanks boxm for the bug report and timo for bisecting the regression.
Conflicts:
tests/select_related_regress/tests.py
Backport of f574220f from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22508#comment:9>
Django
object
Reporter: boxm | Owner: aaugustin
Type: Bug | Status: closed
Component: Database layer | Version: 1.6
(models, ORM) | Resolution: fixed
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"13087020a92afe778970f8445b8add6b167ba084"]:
{{{
#!CommitTicketReference repository=""
revision="13087020a92afe778970f8445b8add6b167ba084"
Added 1.6.5 release note for refs #22508.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22508#comment:10>
Django
object
Reporter: boxm | Owner: aaugustin
Type: Bug | Status: closed
Component: Database layer | Version: 1.6
(models, ORM) | Resolution: fixed
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"b718e292ac7d2e6ea9764aa618f913ead8b8bbb1"]:
{{{
#!CommitTicketReference repository=""
revision="b718e292ac7d2e6ea9764aa618f913ead8b8bbb1"
[1.6.x] Added 1.6.5 release note for refs #22508.
Backport of 13087020a9 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22508#comment:11>
Django
object
Reporter: boxm | Owner: aaugustin
Type: Bug | Status: closed
Component: Database layer | Version: 1.6
(models, ORM) | Resolution: fixed
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"e1f0efc501ad4e8999af51824806c091587fb21c"]:
{{{
#!CommitTicketReference repository=""
revision="e1f0efc501ad4e8999af51824806c091587fb21c"
[1.7.x] Added 1.6.5 release note for refs #22508.
Backport of 13087020a9 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22508#comment:12>