What's the expected behavior of (cached) instances of deleted objects?

[Johannes Dollinger]

QuerySet.delete() currently sets the primary key and all nullable
foreign keys (to deleted objects) of instances passed to signal
handlers to None. No cache is updated.

Model.delete() will do the same, but as these instances are collected
by traversing related object descriptors all reverse o2o field caches
will be updated.

Is this an accidental side effect or desired behavior?

I'm asking because my patch for #7539 currently does not update o2o
caches. But that could be fixed - if really necessary.

Relevant thread: http://groups.google.com/group/django-developers/browse_thread/thread/6e88b6315f489403/

__
Johannes

[Russell Keith-Magee]

On Thu, Nov 26, 2009 at 12:04 AM, Johannes Dollinger
<johannes....@einfallsreich.net> wrote:
> QuerySet.delete() currently sets the primary key and all nullable
> foreign keys (to deleted objects) of instances passed to signal
> handlers to None. No cache is updated.
>
> Model.delete() will do the same, but as these instances are collected
> by traversing related object descriptors all reverse o2o field caches
> will be updated.

Are you sure there is a discrepancy here?

ModelBase.delete() creates a CollectedObjects() structure, then
invokes self._collect_sub_objects() to populate it. It then invokes
delete_objects() to delete the objects.

QuerySet.delete() creates a CollectedObjects() structure, then invokes
obj._collect_sub_objects() on each of the objects in the queryset to
populate. It then invokes delete_objects() to delete the objects.

As far as I can make out, both populate the object caches the same way.

> Is this an accidental side effect or desired behavior?

If you are deleting a single object X, X.delete() deletes the object,
so the values in the cache are irrelevant. Similarly, if you delete a
queryset Y, all the items in Y will be deleted, and all the values in
the cache will be irrelevant.

In the case of an object Z that points at X (or an object in Y), but
doesn't result in a cacscading delete, the cache in Z should be
cleared. This is already done for nullable keys; I imagine your code
will need to implement something similar to handle non-cascading
cases.

Yours,
Russ Magee %-)

[Johannes Dollinger]

Am 26.11.2009 um 03:47 schrieb Russell Keith-Magee:

> On Thu, Nov 26, 2009 at 12:04 AM, Johannes Dollinger
> <johannes....@einfallsreich.net> wrote:
>> QuerySet.delete() currently sets the primary key and all nullable
>> foreign keys (to deleted objects) of instances passed to signal
>> handlers to None. No cache is updated.
>>
>> Model.delete() will do the same, but as these instances are collected
>> by traversing related object descriptors all reverse o2o field caches
>> will be updated.
>
> Are you sure there is a discrepancy here?
>
> ModelBase.delete() creates a CollectedObjects() structure, then
> invokes self._collect_sub_objects() to populate it. It then invokes
> delete_objects() to delete the objects.
>
> QuerySet.delete() creates a CollectedObjects() structure, then invokes
> obj._collect_sub_objects() on each of the objects in the queryset to
> populate. It then invokes delete_objects() to delete the objects.

With QuerySet.delete() all instances traversed by
_collect_sub_objects() are only accessible from signal handlers, there
will be no surprises.
But with Model.delete() you can possible access the same instances
through reverse o2o descriptors.
Here is an example: http://dpaste.com/hold/125343/

> As far as I can make out, both populate the object caches the same
> way.
>
>> Is this an accidental side effect or desired behavior?
>
> If you are deleting a single object X, X.delete() deletes the object,
> so the values in the cache are irrelevant. Similarly, if you delete a
> queryset Y, all the items in Y will be deleted, and all the values in
> the cache will be irrelevant.
>
> In the case of an object Z that points at X (or an object in Y), but
> doesn't result in a cacscading delete, the cache in Z should be
> cleared. This is already done for nullable keys; I imagine your code
> will need to implement something similar to handle non-cascading
> cases.

Unconditionally for all instances? That sounds like solution for #17
is needed. Currently, nullable keys are only updated for instances
that are collected in _collect_sub_objects().
As described above, these are only accessible from signals handlers
(this is covered by my patch) and reverse o2o descriptors (hence my
question).

NB: The tests pass fine, even without o2o cache updates.
__
Johannes

[Russell Keith-Magee]

On Thu, Nov 26, 2009 at 5:39 PM, Johannes Dollinger
<johannes....@einfallsreich.net> wrote:
>
> Am 26.11.2009 um 03:47 schrieb Russell Keith-Magee:
>
>> On Thu, Nov 26, 2009 at 12:04 AM, Johannes Dollinger
>> <johannes....@einfallsreich.net> wrote:
>>> QuerySet.delete() currently sets the primary key and all nullable
>>> foreign keys (to deleted objects) of instances passed to signal
>>> handlers to None. No cache is updated.
>>>
>>> Model.delete() will do the same, but as these instances are collected
>>> by traversing related object descriptors all reverse o2o field caches
>>> will be updated.
>>
>> Are you sure there is a discrepancy here?
>>
>> ModelBase.delete() creates a CollectedObjects() structure, then
>> invokes self._collect_sub_objects() to populate it. It then invokes
>> delete_objects() to delete the objects.
>>
>> QuerySet.delete() creates a CollectedObjects() structure, then invokes
>> obj._collect_sub_objects() on each of the objects in the queryset to
>> populate. It then invokes delete_objects() to delete the objects.
>
> With QuerySet.delete() all instances traversed by
> _collect_sub_objects() are only accessible from signal handlers, there
> will be no surprises.
> But with Model.delete() you can possible access the same instances
> through reverse o2o descriptors.
> Here is an example: http://dpaste.com/hold/125343/

Looks to me like you've found some edge case errors. If I'm reading
the test case right, the o2o field isn't nullable, so you should be
getting None for c.pk, and DNE on every next/prev request.

>> As far as I can make out, both populate the object caches the same
>> way.
>>
>>> Is this an accidental side effect or desired behavior?
>>
>> If you are deleting a single object X, X.delete() deletes the object,
>> so the values in the cache are irrelevant. Similarly, if you delete a
>> queryset Y, all the items in Y will be deleted, and all the values in
>> the cache will be irrelevant.
>>
>> In the case of an object Z that points at X (or an object in Y), but
>> doesn't result in a cacscading delete, the cache in Z should be
>> cleared. This is already done for nullable keys; I imagine your code
>> will need to implement something similar to handle non-cascading
>> cases.
>
> Unconditionally for all instances? That sounds like solution for #17
> is needed. Currently, nullable keys are only updated for instances
> that are collected in _collect_sub_objects().

No - not unconditionally. I'm only talking about instances where you
get access to an object through a descriptor, and subsequently the
object that provided that descriptor is deleted - essentially the case
you have described here

Yours,
Russ Magee %-)

[Johannes Dollinger]

There's currently no code that would update forward foreign key caches.
So it's the edge case that does the updates (reverse o2o) and the
general case just leaves your object graph alone (including stale
caches).

class A(models.Model): pass
class B(models.Model): a = models.ForeignKey(A)

a = A.objects.create()
b = B.objects.create(a=a)
b.a.delete()

Although both instances have been deleted, `b.a` will still be
accessible.
__
Johannes