Opened 5 years ago

Closed 5 years ago

Last modified 4 years ago

#13227 closed (fixed)

Query cloning fails with models having Lock as instance member

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

Description

Here is a basic test case:

import threading
from django.db import models

class BaseObject(models.Model):
    status      = models.CharField(max_length=10)
    foreign     = models.ForeignKey('MyModel')

class MyModel(models.Model):
    name = models.CharField(max_length=10)

    def __init__(self, *args, **kwargs):
        super(MyModel, self).__init__(*args, **kwargs)
        self.checkout_lock = threading.Lock()

Now as soon as a query with MyModel instance has to be cloned, this fails.

from testapp.models import MyModel, BaseObject
mm = MyModel(name='Test1')
mm.save()
bo = BaseObject(status='a',foreign=mm)
bo.save()

print BaseObject.objects.filter(foreign=mm).order_by('foreign__name')
(full traceback here)
Error: un(deep)copyable object of type <type 'thread.lock'>

Change History (11)

comment:1 Changed 5 years ago by claudep

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

Also note that this doesn't happen with 1.1

comment:2 Changed 5 years ago by gav

  • milestone set to 1.2

If this doesn't happen in 1.1, but does in 1.2, the should this be marked as a regression for 1.2 release? Marking the milestone so a core dev will notice and decide, since that fits the criteria from other tickets.

comment:3 Changed 5 years ago by Alex

  • Triage Stage changed from Unreviewed to Accepted
  • Version changed from 1.2-beta to SVN

comment:4 Changed 5 years ago by russellm

I'm looking at this right now; it's a problem with the way db_prep_save is used on foreign keys. Should have a patch soon.

comment:5 Changed 5 years ago by russellm

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

(In [12865]) Fixed #13227 -- Modified ForeignKeys to fully honor the db_prep/prep separation introduced by multidb. This was required to ensure that model instances aren't deepcopied as a result of being involved in a filter clause. Thanks to claudep for the report, and Alex Gaynor for the help on the patch.

comment:6 Changed 5 years ago by claudep

  • Resolution fixed deleted
  • Status changed from closed to reopened

I'm so sorry, but I think this is not completely resolved. Here is an updated test case where the same error appear. Model is unchanged.

from testapp.models import MyModel, BaseObject
mm = MyModel(name='Test1')
mm.save()
bo = BaseObject(status='a',foreign=mm)
bo.save()

mms = MyModel.objects.all()
mms_id_list = [m.id for m in mms]
print BaseObject.objects.filter(foreign__in=mms).order_by('foreign__name')

Note the mms_id_list creation (apparently unrelated to the problem) which access the id of MyModel objects. Without this line, the query is successfull.

comment:7 Changed 5 years ago by russellm

The issue is that by evaluating mms_id_list (which evaluates the mms queryset), you populate the mms queryset cache with object instances; when you then try to evaluate the BaseObject query, you need to be able to clone those instances.

On closer inspection, it looks like the original problem was also present in 1.1... but this problem *isn't*, so it's a regression. However I also need to backport r12865 to 1.1.

comment:8 Changed 5 years ago by russellm

(In [12963]) [1.1.X] Refs #13227 -- Partial backport of r12865; backported the changes to Where tree cloning logic to ensure that unclonable objects in a where() clause don't break querying.

comment:9 Changed 5 years ago by russellm

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

(In [12970]) Fixed #13227 -- Ensure that the query cache is flushed when a QuerySet is deepcopied, avoiding problems when an evaluated queryset is used as a subquery. Thanks to claudep for the report.

comment:10 Changed 5 years ago by claudep

Thanks Russell, it's fine now!

comment:11 Changed 4 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 deleted

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