[Django] #23370: defer() with inherited models fails
Django
----------------------------------------------+--------------------
Reporter: dfunckt | Owner: nobody
Type: Bug | Status: new
Component: Database layer (models, ORM) | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+--------------------
Let's say I have two models: {{{Post(models.Model)}}} and
{{{Article(Post)}}} and some data.
Now if I evaluate the query set
{{{Post.objects.select_related('article').defer('article__somefield')}}}
it will fail with:
{{{
...
File "django/db/models/query.py", line 1457, in get_cached_row
if (fields[pk_idx] is None or
IndexError: tuple index out of range
}}}
This prevents us from using {{{defer()}}} to optimise our queries to defer
loading of large TEXT fields when showing lists of posts.
This is present in master [a81af7f49de7ff3f51f111de28ed3a682f67ea89] and
(with a similar traceback) since at least Django 1.6.
I have attached a test project that demonstrates the bug. Add the project
to path and run:
{{{
$ ./manage.py migrate
$ ./manage.py testdefer
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23370>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: dfunckt | Owner: nobody
Type: Bug | Status: new
(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 dfunckt):
* cc: akiskesoglou@… (added)
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/23370#comment:1>
Django
Reporter: dfunckt | Owner: nobody
Type: Bug | Status: new
(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 timgraham):
* stage: Unreviewed => Accepted
Comment:
Reproduced with the attached test case for Django's test suite.
--
Ticket URL: <https://code.djangoproject.com/ticket/23370#comment:2>
Django
Reporter: dfunckt | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by gcbirzan):
This is the same bug as #23270. Or, rather, the same fix.
--
Ticket URL: <https://code.djangoproject.com/ticket/23370#comment:3>
Django
Reporter: dfunckt | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by dfunckt):
Just tested Vladimiroff's branch but doesn't fix this issue. The bugs seem
similar though.
--
Ticket URL: <https://code.djangoproject.com/ticket/23370#comment:4>
Django
Reporter: dfunckt | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Needs tests: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by dfunckt):
* has_patch: 0 => 1
* easy: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/23370#comment:5>
Django
Reporter: dfunckt | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 1 |
-------------------------------------+-------------------------------------
Comment (by dfunckt):
The fix is easy, but unrelated to #23270.
PR: https://github.com/django/django/pull/3137
--
Ticket URL: <https://code.djangoproject.com/ticket/23370#comment:6>
Django
Reporter: dfunckt | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 1 |
-------------------------------------+-------------------------------------
Comment (by dfunckt):
I'd also like to backport the fix to the 1.6 branch. Is it possible?
--
Ticket URL: <https://code.djangoproject.com/ticket/23370#comment:7>
Django
Reporter: dfunckt | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 1 |
-------------------------------------+-------------------------------------
Comment (by akaariai):
Unless this is a regression in 1.6, then the backporting rules says that
we shouldn't backport this.
--
Ticket URL: <https://code.djangoproject.com/ticket/23370#comment:8>
Django
Reporter: dfunckt | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 1 |
-------------------------------------+-------------------------------------
Comment (by dfunckt):
The bug must have been introduced in this
[https://github.com/django/django/commit/f51e409a5fb34020e170494320a421503689aea0
#diff-5b0dda5eb9a242c15879dc9cd2121379L1350 commit], which I believe was
made on the road to 1.6. What do you think? I'm not trying to put
pressure, but *if* there's going to be another 1.6 release, it'd be nice
to have this included.
--
Ticket URL: <https://code.djangoproject.com/ticket/23370#comment:9>
Django
Reporter: dfunckt | Owner: nobody
Type: Bug | Status: new
(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 timgraham):
* easy: 1 => 0
* stage: Accepted => Ready for checkin
Comment:
Check if the test passes on 1.5. Ready for final ORM review. If it's
backported, it'll need release notes though.
--
Ticket URL: <https://code.djangoproject.com/ticket/23370#comment:10>
Django
Reporter: dfunckt | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by bmispelon):
I also confirm that this is a regression from 1.5 to 1.6 which would
warrant the backporting.
Using git bisect, I found that the commit that introduced the regression
is actually 6ebf115206289bce8f3d86318871faac13d6e835.
Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/23370#comment:11>
Django
Reporter: dfunckt | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by dfunckt):
Updated the PR based on timgraham's comment. Ran tests -- all pass.
I'll run the test on 1.5 and create a separate PR for 1.6 and let you
know.
--
Ticket URL: <https://code.djangoproject.com/ticket/23370#comment:12>
Django
Reporter: dfunckt | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
Probably don't need a separate PR. We have scripts to do the backporting.
--
Ticket URL: <https://code.djangoproject.com/ticket/23370#comment:13>
Django
Reporter: dfunckt | Owner: nobody
Type: Bug | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by dfunckt):
Updated [https://github.com/django/django/pull/3137 PR] with a new commit
with release notes for 1.6.7.
--
Ticket URL: <https://code.djangoproject.com/ticket/23370#comment:14>
Django
Reporter: dfunckt | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: master
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"6613ea6e3ff2597c04db35ef885562e95c1ef012"]:
{{{
#!CommitTicketReference repository=""
revision="6613ea6e3ff2597c04db35ef885562e95c1ef012"
Fixed #23370 -- defer() + select_related() crashed with inherited models.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23370#comment:15>
Django
Reporter: dfunckt | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: master
(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:"b877697472073454423a0cdff695387d9039b464"]:
{{{
#!CommitTicketReference repository=""
revision="b877697472073454423a0cdff695387d9039b464"
[1.6.x] Fixed #23370 -- defer() + select_related() crashed with inherited
models.
Backport of 6613ea6e3f from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23370#comment:16>
Django
Reporter: dfunckt | Owner: nobody
Type: Bug | Status: closed
Component: Database layer | Version: master
(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:"3297f9e1ad97da1e63ca827239663d6b33949037"]:
{{{
#!CommitTicketReference repository=""
revision="3297f9e1ad97da1e63ca827239663d6b33949037"
[1.7.x] Fixed #23370 -- defer() + select_related() crashed with inherited
models.
Backport of 6613ea6e3f from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23370#comment:17>