Opened 10 months ago

Closed 10 months ago

Last modified 10 months ago

#23370 closed Bug (fixed)

defer() with inherited models fails

Reported by: dfunckt Owned by: nobody
Component: Database layer (models, ORM) Version: master
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 dfunckt 10 months ago.
Test project
23370-test.diff (1.0 KB) - added by timgraham 10 months ago.

Download all attachments as: .zip

Change History (19)

Changed 10 months ago by dfunckt

Test project

comment:1 Changed 10 months ago by dfunckt

  • Cc akiskesoglou@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Changed 10 months ago by timgraham

comment:2 Changed 10 months ago by timgraham

  • Triage Stage changed from Unreviewed to Accepted

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

comment:3 Changed 10 months ago by gcbirzan

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

comment:4 Changed 10 months ago by dfunckt

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

comment:5 Changed 10 months ago by dfunckt

  • Easy pickings set
  • Has patch set

comment:6 Changed 10 months ago by dfunckt

The fix is easy, but unrelated to #23270.

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

comment:7 Changed 10 months ago by dfunckt

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

comment:8 Changed 10 months ago by akaariai

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

comment:9 Changed 10 months ago by dfunckt

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 10 months ago by timgraham

  • Easy pickings unset
  • Triage Stage changed from Accepted to Ready 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 10 months ago 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.

comment:12 Changed 10 months ago 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.

comment:13 Changed 10 months ago by timgraham

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

comment:14 Changed 10 months ago by dfunckt

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

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

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

In 6613ea6e3ff2597c04db35ef885562e95c1ef012:

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

comment:16 Changed 10 months 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 10 months 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