Opened 14 years ago

Closed 14 years ago

Last modified 12 years ago

#13328 closed (fixed)

Cannot pickle a queryset with filter on field with callable default datetime.datetime.now

Reported by: Brandon Konkle Owned by: nobody
Component: Database layer (models, ORM) Version: 1.2-beta
Severity: Keywords: pickle, queryset, datetime__lte
Cc: brandonkonkle@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Karen Tracey)

We're caching with memcached, and using django.core.cache to directly set cache values. When we try to cache a queryset of blog entries with a datetime__lte filter, we get a TypeError telling us that a string or Unicode object was expected, but a NoneType was received instead. I'm attaching an abbreviated example of the model and manager we're using, and the manage.py shell steps taken to reproduce the error along with the traceback it provides.

The error is related to line 6 of the attached models.py file - if I remove the release_date__lte=datetime.datetime.now() filter, the cache.set works without an issue.

Attachments (7)

interacitve_output.txt (1.7 KB ) - added by Brandon Konkle 14 years ago.
iPython interactive interpreter output, including traceback
models.py (843 bytes ) - added by Brandon Konkle 14 years ago.
An abbreviated models.py which, when used, reproduces the error.
13328_test.diff (1.5 KB ) - added by Karen Tracey 14 years ago.
patch_13328.diff (610 bytes ) - added by vbabiy 14 years ago.
patch
patch_v2_13328.diff (1.1 KB ) - added by vbabiy 14 years ago.
Second version of my patch
patch_v3_13328_with_tests.diff (3.0 KB ) - added by vbabiy 14 years ago.
Thrid version of my patch
patch_v3_13328_with_tests.2.diff (3.0 KB ) - added by vbabiy 14 years ago.
Thrid version of my patch

Download all attachments as: .zip

Change History (26)

by Brandon Konkle, 14 years ago

Attachment: interacitve_output.txt added

iPython interactive interpreter output, including traceback

by Brandon Konkle, 14 years ago

Attachment: models.py added

An abbreviated models.py which, when used, reproduces the error.

comment:1 by Karen Tracey, 14 years ago

Description: modified (diff)

Fixed formatting. Please use preview.

comment:2 by Brandon Konkle, 14 years ago

Cc: brandonkonkle@… added

comment:3 by Karen Tracey, 14 years ago

Component: Cache systemDatabase layer (models, ORM)
Summary: Pickling a queryset with a datetime__lte filter causes a TypeErrorCannot pickle a queryset with filter on field with callable default datetime.datetime.now
Triage Stage: UnreviewedAccepted

The root of the problem is that the field being filtered has a default value of datetime.datetime.now. Change in behavior was introduced with mutlidb (r11952). I'll attach a testcase that passes prior to r11952 and fails after.

by Karen Tracey, 14 years ago

Attachment: 13328_test.diff added

comment:4 by Karen Tracey, 14 years ago

Note the TypeError: expected string or Unicode object, NoneType found message in the attached interactive output is the cPickle (used by the cache code if available) version of plain pickle error below:

No fixtures found.
test_datetime_callable_default_all (regressiontests.queryset_pickle.tests.PickleabilityTestCase) ... ok
test_datetime_callable_default_filter (regressiontests.queryset_pickle.tests.PickleabilityTestCase) ... ERROR
test_related_field (regressiontests.queryset_pickle.tests.PickleabilityTestCase) ... ok

======================================================================
ERROR: test_datetime_callable_default_filter (regressiontests.queryset_pickle.tests.PickleabilityTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/kmt/django/trunk/tests/regressiontests/queryset_pickle/tests.py", line 20, in test_datetime_callable_default_filter
    self.assert_pickles(Happening.objects.filter(when=datetime.datetime.now()))
  File "/home/kmt/django/trunk/tests/regressiontests/queryset_pickle/tests.py", line 10, in assert_pickles
    self.assertEqual(list(pickle.loads(pickle.dumps(qs))), list(qs))
  File "/usr/lib/python2.5/pickle.py", line 1366, in dumps
    Pickler(file, protocol).dump(obj)
  File "/usr/lib/python2.5/pickle.py", line 224, in dump
    self.save(obj)
  File "/usr/lib/python2.5/pickle.py", line 331, in save
    self.save_reduce(obj=obj, *rv)
  File "/usr/lib/python2.5/pickle.py", line 419, in save_reduce
    save(state)
  File "/usr/lib/python2.5/pickle.py", line 286, in save
    f(self, obj) # Call unbound method with explicit self
  File "/usr/lib/python2.5/pickle.py", line 649, in save_dict
    self._batch_setitems(obj.iteritems())
  File "/usr/lib/python2.5/pickle.py", line 663, in _batch_setitems
    save(v)
  File "/usr/lib/python2.5/pickle.py", line 331, in save
    self.save_reduce(obj=obj, *rv)
  File "/usr/lib/python2.5/pickle.py", line 419, in save_reduce
    save(state)
  File "/usr/lib/python2.5/pickle.py", line 286, in save
    f(self, obj) # Call unbound method with explicit self
  File "/usr/lib/python2.5/pickle.py", line 649, in save_dict
    self._batch_setitems(obj.iteritems())
  File "/usr/lib/python2.5/pickle.py", line 663, in _batch_setitems
    save(v)
  File "/usr/lib/python2.5/pickle.py", line 331, in save
    self.save_reduce(obj=obj, *rv)
  File "/usr/lib/python2.5/pickle.py", line 419, in save_reduce
    save(state)
  File "/usr/lib/python2.5/pickle.py", line 286, in save
    f(self, obj) # Call unbound method with explicit self
  File "/usr/lib/python2.5/pickle.py", line 649, in save_dict
    self._batch_setitems(obj.iteritems())
  File "/usr/lib/python2.5/pickle.py", line 663, in _batch_setitems
    save(v)
  File "/usr/lib/python2.5/pickle.py", line 286, in save
    f(self, obj) # Call unbound method with explicit self
  File "/usr/lib/python2.5/pickle.py", line 600, in save_list
    self._batch_appends(iter(obj))
  File "/usr/lib/python2.5/pickle.py", line 615, in _batch_appends
    save(x)
  File "/usr/lib/python2.5/pickle.py", line 331, in save
    self.save_reduce(obj=obj, *rv)
  File "/usr/lib/python2.5/pickle.py", line 419, in save_reduce
    save(state)
  File "/usr/lib/python2.5/pickle.py", line 286, in save
    f(self, obj) # Call unbound method with explicit self
  File "/usr/lib/python2.5/pickle.py", line 649, in save_dict
    self._batch_setitems(obj.iteritems())
  File "/usr/lib/python2.5/pickle.py", line 663, in _batch_setitems
    save(v)
  File "/usr/lib/python2.5/pickle.py", line 286, in save
    f(self, obj) # Call unbound method with explicit self
  File "/usr/lib/python2.5/pickle.py", line 600, in save_list
    self._batch_appends(iter(obj))
  File "/usr/lib/python2.5/pickle.py", line 615, in _batch_appends
    save(x)
  File "/usr/lib/python2.5/pickle.py", line 286, in save
    f(self, obj) # Call unbound method with explicit self
  File "/usr/lib/python2.5/pickle.py", line 562, in save_tuple
    save(element)
  File "/usr/lib/python2.5/pickle.py", line 331, in save
    self.save_reduce(obj=obj, *rv)
  File "/usr/lib/python2.5/pickle.py", line 419, in save_reduce
    save(state)
  File "/usr/lib/python2.5/pickle.py", line 286, in save
    f(self, obj) # Call unbound method with explicit self
  File "/usr/lib/python2.5/pickle.py", line 649, in save_dict
    self._batch_setitems(obj.iteritems())
  File "/usr/lib/python2.5/pickle.py", line 663, in _batch_setitems
    save(v)
  File "/usr/lib/python2.5/pickle.py", line 331, in save
    self.save_reduce(obj=obj, *rv)
  File "/usr/lib/python2.5/pickle.py", line 419, in save_reduce
    save(state)
  File "/usr/lib/python2.5/pickle.py", line 286, in save
    f(self, obj) # Call unbound method with explicit self
  File "/usr/lib/python2.5/pickle.py", line 649, in save_dict
    self._batch_setitems(obj.iteritems())
  File "/usr/lib/python2.5/pickle.py", line 663, in _batch_setitems
    save(v)
  File "/usr/lib/python2.5/pickle.py", line 286, in save
    f(self, obj) # Call unbound method with explicit self
  File "/usr/lib/python2.5/pickle.py", line 748, in save_global
    (obj, module, name))
PicklingError: Can't pickle <built-in method now of type object at 0xb7da4500>: it's not found as __main__.now

----------------------------------------------------------------------
Ran 3 tests in 0.117s

FAILED (errors=1)

in reply to:  4 comment:5 by Brandon Konkle, 14 years ago

That makes MUCH more sense now - the Unicode/string error had me thoroughly confused.

comment:6 by vbabiy, 14 years ago

Here is my first go at a patch for Django.

My thinking was that if you are serializing the queryset we can just remove the default off the datetime field to allow pickling the queryset.

by vbabiy, 14 years ago

Attachment: patch_13328.diff added

patch

comment:7 by vbabiy, 14 years ago

Has patch: set

by vbabiy, 14 years ago

Attachment: patch_v2_13328.diff added

Second version of my patch

by vbabiy, 14 years ago

Thrid version of my patch

by vbabiy, 14 years ago

Thrid version of my patch

comment:8 by Russell Keith-Magee, 14 years ago

Resolution: fixed
Status: newclosed

(In [12977]) Fixed #13328 -- Added a getstate/setstate pair to fields so that callable default values aren't pickled. Thanks to bkonkle for the report, and Vitaly Babiy for the patch.

comment:9 by Patryk Zawadzki, 14 years ago

Resolution: fixed
Status: closedreopened

Actually I think [12977] and [13005] should be reverted as the current solution:

  1. violates the layer separation (fields being self-contained and not caring about the surrounding classes)
  2. does the ugly "find an older definition of myself and try to guess what the default was" dance
  3. breaks in horrible ways when fields are used by anything other than django.db.models.Model subclasses
  4. breaks apps such as transmeta (yes, I know transmeta is not exactly loved by the devs, I just mention it here for completness) that rely on being able to copy() a field in the metaclass

Class and instance methods are unpickleable in Python. Be it Django or not. Period.

What happens here is that datetime.datetime.now is a @staticmethod. You can check that datetime.datetime is an object, not a module. Python only knows how to pickle functions it can reference by a module path. Instance methods are not reachable without an instance and static methods don't know their own python path because in reality they all point to a temporary function created inside the @staticmethod decorator (well, that's how decorators work).

A real fix is to either teach Python how to pickle static methods or use a function instead of a method. Functions are pickleable and work fine:

def current_time():
    return datetime.datetime.now()

class Foo(models.Model):
    some_field = models.DateTimeField(default=current_time)

comment:10 by Russell Keith-Magee, 14 years ago

I'm in complete agreement that the current solution isn't clean. The report from #13392 is another example of where the complication of a field knowing it's model can go badly wrong.

However, your solution isn't really viable. Like it or not, there is *plenty* of code out there that uses "default=datetime.now". Saying that this isn't allowed would be a *massive* backwards incompatibility.

comment:11 by Patryk Zawadzki, 14 years ago

Russel:

Keep in mind it only breaks if you try to pickle it. I imagine most people don't pickle evaluated querysets.

If we really need a hack to solve it then it would be easier to detect datetime.datetime.now in the Field constructor and replace it with a reference to, say, django.db.utils.current_time that is a regular module-level function. At the same time raise a deprecation warning.

comment:12 by Patryk Zawadzki, 14 years ago

To be clear:

Where I say django.db.utils.current_time I actually mean to introduce safe replacements for common unpickleable methods (maybe something like django.pickleable.today or safe_today would be better names).

comment:13 by Russell Keith-Magee, 14 years ago

It isn't just evaluate querysets,though. It's *any* queryset with a callable default -- see the tests added in r12977 for an example.

On top of that, the docs clearly say that default can be a callable object, so we need to be able to handle *any* callable.

Furthermore, this is a behavior that was legal in 1.1 (due to differences in the way queries were constructed), so we need to restore it for 1.2.

comment:14 by Patryk Zawadzki, 14 years ago

I assume it wouldn't be possible to handle the most common case in code (datetime.*) and document the rest as backwards-incompatible? :)

Then maybe it would be easier to not pickle the whole model at all when pickling the queryset? I'm sure it wasn't possible to pickle datetime.now or, even worse, a lambda at any point in time so maybe it would be possible to make the post-multidb queryset not pickle the model contents (the model is guaranteed to be there when unpickling).

comment:15 by Russell Keith-Magee, 14 years ago

I suspect you're probably correct that the solution is to look at cleaning up the object in a query that is being pickled. My original hope was that by solving the problem a little higher up, we might be able to avoid other pickling problems that haven't been reported, but it looks like I've just made more work for myself instead /sigh... :-)

comment:16 by Russell Keith-Magee, 14 years ago

Resolution: fixed
Status: reopenedclosed

(In [13013]) Fixed #13328 -- Ensured that querysets on models with callable defaults can be pickled. No, really this time. Thanks to Alex for his help brainstorming the solution.

comment:17 by Patryk Zawadzki, 14 years ago

Russel:

Thanks for fixing the main issue but I'm not sure the tests in that commit are correct or that they actually test anything related to this bug :)

comment:18 by Russell Keith-Magee, 14 years ago

Patrys - If you're referring to the 'number2=1' duplication, Karen fixed that in [13014].

comment:19 by Jacob, 12 years ago

milestone: 1.2

Milestone 1.2 deleted

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