Code

Opened 2 years ago

Closed 17 months ago

#17565 closed Bug (fixed)

Cache back-references on OneToOne fields

Reported by: shai Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords: OneToOneField, OneToOne
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Today, back-references on OneToOneField's are not cached. In more detail, assume:

    class A(model):
        pass

    class B(model):
        a = OneToOneField(A)

    a1 = A.objects.get(...) # assume a1 has a related B record
    b1 = B.objects.get(...)

Today, each of the expressions a1.b.a and b1.a.b generates two database queries, and as a result, both (a1.b.a is a1) and (b1.a.b is b1) are false. This is surprising and can be both a performance issue and a correctness issue -- therefore classified as "Bug", although it is borderline "optimization".

(Of course, nobody in their right mind actually writes code like a1.b.a; the issue arises e.g. by function f taking a1 and calling g(a1.b), where g does something with b.a).

Attached is a patch with tests that failed on trunk r17377.

Attachments (2)

test-1to1-backref-cache.patch (1.3 KB) - added by shai 2 years ago.
Patch adding regression tests for OneToOneField back-reference caching
add-1to1-backref-cache.patch (3.5 KB) - added by shai 2 years ago.
And here is the fix. This makes the tests attached above pass, and doesn't seem to break any others. Also, a similar solution has been exercised with no problems in a Django-1.2 based solution.

Download all attachments as: .zip

Change History (6)

Changed 2 years ago by shai

Patch adding regression tests for OneToOneField back-reference caching

Changed 2 years ago by shai

And here is the fix. This makes the tests attached above pass, and doesn't seem to break any others. Also, a similar solution has been exercised with no problems in a Django-1.2 based solution.

comment:1 Changed 2 years ago by shai

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

I've just checked: both patches also apply and work against branch 1.3.X@17370. They don't break any tests (one test, regressiontests.generic_views.dates.WeekArchiveViewTests.test_week_view_allow_future, breaks for me on the branch, with or without the patch).

comment:2 Changed 2 years ago by aaugustin

  • Triage Stage changed from Unreviewed to Accepted

Discussed on the mailing list here: http://groups.google.com/group/django-developers/browse_thread/thread/b8dd06eb13ccc8c2

See comments by Adrian and Carl in particular.

comment:3 Changed 17 months ago by shai

Hi,

Just saw this mentioned in the release notes as one of the new features of Django 1.5. I haven't tested it myself, but I guess this bug can be closed now.

comment:4 Changed 17 months ago by aaugustin

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

Indeed. This was reported in several duplicate tickets and I missed this one when I fixed the issue. Thanks for noticing!

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.