#9023 closed (fixed)
OneToOneField delete() problem
Reported by: | TheShark | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Keywords: | OneToOneField delete cascade | |
Cc: | juriejanbotha@…, David Larlet | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
Accessing the automatic reverse relationship on an instance (w.sprocket in the following example) causes some type of caching to happen which results in my Sprocket getting caught up in a cascade delete() of a Widget erroneously. The following example works without the print statement, and my Sprocket s exists at the end. If I 'print w.sprocket' before clearing the relationship, my Sprocket gets deleted along with my Widget when it shouldn't.
class Widget(models.Model): name = models.CharField(max_length=10) def __unicode__(self): return u'%s(%s)'%(self.name,self.pk) class Sprocket(models.Model): name = models.CharField(max_length=10) w = models.OneToOneField(Widget, null=True, blank=True) def __unicode__(self): return u'%s(%s)'%(self.name,self.pk) And the following sequence of operations: w=Widget(name="Some Widget") w.save() s=Sprocket(name="Some Sprocket") s.w=w s.save() assert Widget.objects.all().count()==1 assert Sprocket.objects.all().count()==1 #print w.sprocket s.w=None s.save() w.delete() assert Widget.objects.all().count()==0 assert Sprocket.objects.all().count()==1
Attachments (3)
Change History (16)
comment:1 by , 16 years ago
Component: | Uncategorized → Database layer (models, ORM) |
---|
comment:2 by , 16 years ago
milestone: | → 1.1 |
---|---|
Triage Stage: | Unreviewed → Accepted |
by , 16 years ago
Attachment: | onetoone_cache_clear.diff added |
---|
comment:3 by , 16 years ago
Cc: | added |
---|---|
Has patch: | set |
Needs tests: | set |
Owner: | changed from | to
Patch needs improvement: | set |
I've created a sloppy patch to fix this issue. It's my first whack at working on the DB layer (or making a serious contribution) so please bear with me.
I figured that we need a reference to the SingleRelatedObjectDescriptor that is related to the ReverseSingleRelatedObjectDescriptor for the OnToOne field so that we can access it from the 'set' method of the ReverseSingleRelatedObjectDescriptor to remove the cache from the object that the relation previously pointed to. So I added the 'related_object_descriptor' attribute to the RelatedField object. Not sure if I should rather add the attribute in a more appropriate place, like in RelatedObject or ReverseSingleRelatedObjectDescriptor itself.
comment:5 by , 16 years ago
Cc: | removed |
---|
comment:6 by , 16 years ago
Version: | 1.0 → SVN |
---|
comment:7 by , 16 years ago
Needs tests: | unset |
---|
by , 16 years ago
comment:8 by , 16 years ago
Working patch attached. I'm not sure I'm 100% happy with the approach: in some cases it can make obj.o2o = None
perform a database hit, which is annoying and counter-intuitive.
comment:9 by , 16 years ago
We can remove the extra SQL query by essentially unrolling the getattr call. We only need to update the related object if the obj is already loaded, so can check this by poking around in dict.
comment:10 by , 16 years ago
Owner: | changed from | to
---|
comment:11 by , 16 years ago
Cc: | added |
---|
comment:12 by , 16 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
First sloppy version of the patch (needs feedback)