Opened 6 years ago

Closed 6 years ago

Last modified 8 months ago

#18416 closed Bug (wontfix)

Query fails when SimpleLazyObject evaluates to None

Reported by: Bouke Haarsma Owned by: nobody
Component: Database layer (models, ORM) Version: 1.4
Severity: Normal Keywords:
Cc: dougvanhorn Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


Relevant excerpt from model definition:

class Event(models.Model):
    season = models.ForeignKey(Season)

Expected result:

>>> Event.objects.filter(season=None)

Actual result:

>>> from django.utils.functional import SimpleLazyObject
>>> lazy = SimpleLazyObject(lambda: None)
>>> lazy
<django.utils.functional.SimpleLazyObject object at 0x10203cd90>
>>> print lazy
>>> Event.objects.filter(season=lazy)
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "django/db/models/", line 143, in filter
    return self.get_query_set().filter(*args, **kwargs)
  File "django/db/models/", line 621, in filter
    return self._filter_or_exclude(False, *args, **kwargs)
  File "django/db/models/", line 639, in _filter_or_exclude
    clone.query.add_q(Q(*args, **kwargs))
  File "django/db/models/sql/", line 1250, in add_q
    can_reuse=used_aliases, force_having=force_having)
  File "django/db/models/sql/", line 1185, in add_filter
  File "django/db/models/sql/", line 69, in add
    value = obj.prepare(lookup_type, value)
  File "django/db/models/sql/", line 320, in prepare
    return self.field.get_prep_lookup(lookup_type, value)
  File "django/db/models/fields/", line 137, in get_prep_lookup
    return self._pk_trace(value, 'get_prep_lookup', lookup_type)
  File "django/db/models/fields/", line 210, in _pk_trace
    v = getattr(field, prep_func)(lookup_type, v, **kwargs)
  File "django/db/models/fields/", line 310, in get_prep_lookup
    return self.get_prep_value(value)
  File "django/db/models/fields/", line 537, in get_prep_value
    return int(value)
TypeError: int() argument must be a string or a number, not 'SimpleLazyObject'

The SimpleLazyObject initialization is somewhat more complicated then depicted above, but it can evaluate to None. However, if it evaluates to None, it cannot be used as-is in the query and results in the exception as listed above.

Change History (7)

comment:1 Changed 6 years ago by Luke Plant

Resolution: wontfix
Status: newclosed

WONTFIX, I'm afraid. There is nothing we could do apart from putting a special case for SimpleLazyObject in get_prep_value() where the is None test gets done, which would be horrible.

It is Simple Lazy Object after all.

comment:2 Changed 6 years ago by fabiosantosart@…

And yet, when you accidentally pass None, django gives you that really tall call stack with an unrelated error message at the bottom. Couldn't a better message be issued?

comment:3 Changed 5 years ago by dougvanhorn

I just bumped into this bug. I'm not going to muck with the status, but I wanted to document my thoughts in case anyone else wants to take up the cause on this (I've found it to be a tedious, political process).

The code that raises the exception is this (it's found in most of the Field types):

def get_prep_value(self, value):
    if value is None:
        return None
    return int(value)

It's the value is None that causes the problem. It's an identity test.

If the intent of the if-test is to return None when the supplied value equates to None, then the if-test should be re-written as value == None, which would test for equality. The equality test is slower, though, so you'll likely find argument against using it in this section of the code. Oddly built objects with __eq__ overloaded might also find trouble.

So there may not be much to do here, leaving the burden on the user to figure out what's going on. But I wanted to add some documentation to this bug for future reference.

For what it's worth, I ran into this problem with the following code:

    agreement = Agreement.objects.get(user=request.user)
except Agreement.DoesNotExist, e:
    agreement = Agreement(user=request.user)

I ended up wrapping the view in the login_required decorator, which makes the .user evaluate to something, preventing the bug. But as stated by bouke, the stack is not very helpful.

Last edited 5 years ago by dougvanhorn (previous) (diff)

comment:4 Changed 5 years ago by dougvanhorn

Cc: dougvanhorn added

comment:5 Changed 5 years ago by Jonas H.

Same thing happened to me just now. I believe a common case is request.user in unit tests. Maybe request.user shouldn't be a SimpleLazyObject after all.

comment:6 Changed 2 years ago by Gerben Morsink

Also have this issue now. Now with request.user, but with another object (some model instance) I wanted to be evaluated lazyly.

I'm curious how you fixed it? Completely new LazyObject Class, or is there some quick hack?

I guess I will do a quick test for None before I create the lazyobject. I guess it takes another database call, but I don't know how to do it otherwise.

comment:7 Changed 8 months ago by Ben Youngblut

I also am curious if anyone came up with a good solution for this. I use my own custom user object, and also a organization object that are both attached to the request in middlewares as SimpleLazyObjects that have a None value if they are not logged in or if the logged in user is not part of an organization. Currently attaching to models is a real pain

    organization=request.organization if request.organization else None,

And what is really bad is that if you forget this and don't test with a user without an organization it can easily go unnoticed!

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