Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#23370 closed Bug (fixed)

defer() with inherited models fails

Reported by: Akis Kesoglou Owned by: nobody
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: akiskesoglou@… 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

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

Attachments (2)

testdefer.zip (6.7 KB) - added by Akis Kesoglou 9 years ago.
Test project
23370-test.diff (1.0 KB) - added by Tim Graham 9 years ago.

Download all attachments as: .zip

Change History (19)

Changed 9 years ago by Akis Kesoglou

Attachment: testdefer.zip added

Test project

comment:1 Changed 9 years ago by Akis Kesoglou

Cc: akiskesoglou@… added

Changed 9 years ago by Tim Graham

Attachment: 23370-test.diff added

comment:2 Changed 9 years ago by Tim Graham

Triage Stage: UnreviewedAccepted

Reproduced with the attached test case for Django's test suite.

comment:3 Changed 9 years ago by gcbirzan

This is the same bug as #23270. Or, rather, the same fix.

comment:4 Changed 9 years ago by Akis Kesoglou

Just tested Vladimiroff's branch but doesn't fix this issue. The bugs seem similar though.

comment:5 Changed 9 years ago by Akis Kesoglou

Easy pickings: set
Has patch: set

comment:6 Changed 9 years ago by Akis Kesoglou

The fix is easy, but unrelated to #23270.

PR: https://github.com/django/django/pull/3137

comment:7 Changed 9 years ago by Akis Kesoglou

I'd also like to backport the fix to the 1.6 branch. Is it possible?

comment:8 Changed 9 years ago by Anssi Kääriäinen

Unless this is a regression in 1.6, then the backporting rules says that we shouldn't backport this.

comment:9 Changed 9 years ago by Akis Kesoglou

The bug must have been introduced in this 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.

comment:10 Changed 9 years ago by Tim Graham

Easy pickings: unset
Triage Stage: AcceptedReady for checkin

Check if the test passes on 1.5. Ready for final ORM review. If it's backported, it'll need release notes though.

comment:11 Changed 9 years ago by Baptiste Mispelon

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.

comment:12 Changed 9 years ago by Akis Kesoglou

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.

comment:13 Changed 9 years ago by Tim Graham

Probably don't need a separate PR. We have scripts to do the backporting.

comment:14 Changed 9 years ago by Akis Kesoglou

Updated PR with a new commit with release notes for 1.6.7.

comment:15 Changed 9 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In 6613ea6e3ff2597c04db35ef885562e95c1ef012:

Fixed #23370 -- defer() + select_related() crashed with inherited models.

comment:16 Changed 9 years ago by Tim Graham <timograham@…>

In b877697472073454423a0cdff695387d9039b464:

[1.6.x] Fixed #23370 -- defer() + select_related() crashed with inherited models.

Backport of 6613ea6e3f from master

comment:17 Changed 9 years ago by Tim Graham <timograham@…>

In 3297f9e1ad97da1e63ca827239663d6b33949037:

[1.7.x] Fixed #23370 -- defer() + select_related() crashed with inherited models.

Backport of 6613ea6e3f from master

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