Opened 6 years ago

Closed 6 years ago

Last modified 4 years ago

#10695 closed (fixed)

defer() uses the same cached value for all models

Reported by: jbronn Owned by: mtredinnick
Component: Database layer (models, ORM) Version: master
Severity: Keywords: defer qs
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

While trying to get defer() to work with GeoQuerySets I ran into some strange issues. I eventually figured out that that problem was not isolated to GeoDjango. Here a toy model that demonstrates the problem:

from django.db import models

class City(models.Model):
    name = models.CharField(max_length=30)
    def __unicode__(self): 
        return self.name

Creating two cities, and showing what happens when the QuerySet is evaluated:

>>> c = City.objects.create(name='Pueblo')
>>> c = City.objects.create(name='Houston')
>>> City.objects.all()
[<City: Pueblo>, <City: Houston>]

However, when evaluating a QuerySet with the name field deferred, this is what's returned:

>>> City.objects.defer('name')
[<City_Deferred_name: Pueblo>, <City_Deferred_name: Pueblo>]

Notice how it shows "Pueblo" for both records, when the second one should be "Houston." The same thing happens with slicing:

>>> qs = City.objects.defer('name')
>>> qs[0].name
u'Pueblo'
>>> qs[1].name
u'Pueblo'

My gut tells me that when DeferredAttribute is cached it is somehow cached for all of the other attributes. However, I cannot pinpoint how and/or where this is happening.

Attachments (8)

defer.diff (7.6 KB) - added by Alex 6 years ago.
defer_v2.diff (8.4 KB) - added by jbronn 6 years ago.
defer_v3.diff (8.8 KB) - added by jbronn 6 years ago.
defer.2.diff (9.6 KB) - added by Alex 6 years ago.
add tests and code to make it work properly with foreing keys
defer.3.diff (9.6 KB) - added by Alex 6 years ago.
defer.4.diff (9.1 KB) - added by Alex 6 years ago.
defer.5.diff (9.2 KB) - added by Alex 6 years ago.
defer.6.diff (9.2 KB) - added by Alex 6 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 6 years ago by Alex

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Changed 6 years ago by Alex

comment:2 Changed 6 years ago by Alex

  • Has patch set

comment:3 Changed 6 years ago by Alex

An alternate approach would be to put the pk in the class name, however that would mean creating O(n) classes, which strikes me as silly and wasteful of memory, given that this approach works.

Changed 6 years ago by jbronn

Changed 6 years ago by jbronn

Changed 6 years ago by Alex

add tests and code to make it work properly with foreing keys

Changed 6 years ago by Alex

Changed 6 years ago by Alex

comment:4 Changed 6 years ago by mtredinnick

  • Owner changed from mtreddinick to mtredinnick
  • Status changed from new to assigned

I'm not convinced about this patch yet (Alex: work on getting one patch right, instead of submitting 14 really quick ones). All that instance.__dict__ stuff looks a bit ugly. But I see where you're going. Will work on it a bit over the weekend.

Changed 6 years ago by Alex

Changed 6 years ago by Alex

comment:5 Changed 6 years ago by mtredinnick

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

Fixed in r10382.

comment:6 Changed 4 years ago by jacob

  • milestone 1.1 deleted

Milestone 1.1 deleted

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