Django

Code

Ticket #9023 (closed: fixed)

Opened 2 years ago

Last modified 9 months ago

OneToOneField delete() problem

Reported by: TheShark Assigned to: nobody
Milestone: 1.1 Component: Database layer (models, ORM)
Version: SVN Keywords: OneToOneField delete cascade
Cc: juriejanbotha@gmail.com, david Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 1

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

onetoone_cache_clear.diff (2.2 kB) - added by juriejan on 04/08/09 05:42:10.
First sloppy version of the patch (needs feedback)
9023-test.diff (2.7 kB) - added by adurdin on 05/07/09 06:28:45.
Regression test to exemplify the bug.
9023.diff (3.3 kB) - added by jacob on 05/16/09 11:01:38.

Change History

09/15/08 06:00:13 changed by kratorius

  • needs_better_patch changed.
  • component changed from Uncategorized to Database layer (models, ORM).
  • needs_tests changed.
  • needs_docs changed.

02/25/09 20:46:29 changed by jacob

  • stage changed from Unreviewed to Accepted.
  • milestone set to 1.1.

04/08/09 05:42:10 changed by juriejan

  • attachment onetoone_cache_clear.diff added.

First sloppy version of the patch (needs feedback)

04/08/09 05:53:07 changed by juriejan

  • cc set to juriejanbotha@gmail.com, oliver@obeattie.com, cmlenz@gmx.de, gary.wilson@gmail.com, mhf@hex.no, tom@eggdrop.ch, django@apolloner.eu, brosner@gmail.com, tom85@lycos.nl, dan.popovici@gmail.com, django@sparemint.com, lnielsen@eso.org, to, patrick.lauber@divio.ch, mpjung@terreon.de, django@julienphalip.com, remco@diji.biz, barbuzaster@gmail.com, flavio.curella@gmail.com, semente@taurinus.org, stephane@harobed.org.
  • needs_better_patch set to 1.
  • has_patch set to 1.
  • owner changed from nobody to juriejan.
  • needs_tests set to 1.

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.

04/08/09 05:54:38 changed by obeattie

  • cc changed from juriejanbotha@gmail.com, oliver@obeattie.com, cmlenz@gmx.de, gary.wilson@gmail.com, mhf@hex.no, tom@eggdrop.ch, django@apolloner.eu, brosner@gmail.com, tom85@lycos.nl, dan.popovici@gmail.com, django@sparemint.com, lnielsen@eso.org, to, patrick.lauber@divio.ch, mpjung@terreon.de, django@julienphalip.com, remco@diji.biz, barbuzaster@gmail.com, flavio.curella@gmail.com, semente@taurinus.org, stephane@harobed.org to juriejanbotha@gmail.com, cmlenz@gmx.de, gary.wilson@gmail.com, mhf@hex.no, tom@eggdrop.ch, django@apolloner.eu, brosner@gmail.com, tom85@lycos.nl, dan.popovici@gmail.com, django@sparemint.com, lnielsen@eso.org, to, patrick.lauber@divio.ch, mpjung@terreon.de, django@julienphalip.com, remco@diji.biz, barbuzaster@gmail.com, flavio.curella@gmail.com, semente@taurinus.org, stephane@harobed.org.

I didn't subscribe to this :\

04/08/09 05:59:49 changed by juriejan

  • cc changed from juriejanbotha@gmail.com, cmlenz@gmx.de, gary.wilson@gmail.com, mhf@hex.no, tom@eggdrop.ch, django@apolloner.eu, brosner@gmail.com, tom85@lycos.nl, dan.popovici@gmail.com, django@sparemint.com, lnielsen@eso.org, to, patrick.lauber@divio.ch, mpjung@terreon.de, django@julienphalip.com, remco@diji.biz, barbuzaster@gmail.com, flavio.curella@gmail.com, semente@taurinus.org, stephane@harobed.org to juriejanbotha@gmail.com.

04/08/09 06:04:50 changed by juriejan

  • version changed from 1.0 to SVN.

05/07/09 06:28:45 changed by adurdin

  • attachment 9023-test.diff added.

Regression test to exemplify the bug.

05/07/09 06:28:53 changed by adurdin

  • needs_tests deleted.

05/16/09 11:01:38 changed by jacob

  • attachment 9023.diff added.

05/16/09 11:02:50 changed by jacob

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.

06/07/09 17:10:04 changed by Alex

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.

06/08/09 02:40:14 changed by juriejan

  • owner changed from juriejan to nobody.

06/09/09 12:25:41 changed by david

  • cc changed from juriejanbotha@gmail.com to juriejanbotha@gmail.com, david.

06/15/09 09:30:52 changed by russellm

  • status changed from new to closed.
  • resolution set to fixed.

(In [11009]) Fixed #9023 -- Corrected a problem where cached attribute values would cause a delete to cascade to a related object even when the relationship had been set to None. Thanks to TheShark? for the report and test case, and to juriejan and Jacob for their work on the patch.


Add/Change #9023 (OneToOneField delete() problem)




Change Properties
Action