Opened 12 years ago

Closed 12 years ago

Last modified 7 years 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

Description

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
None
>>> Event.objects.filter(season=lazy)
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "django/db/models/manager.py", line 143, in filter
    return self.get_query_set().filter(*args, **kwargs)
  File "django/db/models/query.py", line 621, in filter
    return self._filter_or_exclude(False, *args, **kwargs)
  File "django/db/models/query.py", line 639, in _filter_or_exclude
    clone.query.add_q(Q(*args, **kwargs))
  File "django/db/models/sql/query.py", line 1250, in add_q
    can_reuse=used_aliases, force_having=force_having)
  File "django/db/models/sql/query.py", line 1185, in add_filter
    connector)
  File "django/db/models/sql/where.py", line 69, in add
    value = obj.prepare(lookup_type, value)
  File "django/db/models/sql/where.py", line 320, in prepare
    return self.field.get_prep_lookup(lookup_type, value)
  File "django/db/models/fields/related.py", line 137, in get_prep_lookup
    return self._pk_trace(value, 'get_prep_lookup', lookup_type)
  File "django/db/models/fields/related.py", line 210, in _pk_trace
    v = getattr(field, prep_func)(lookup_type, v, **kwargs)
  File "django/db/models/fields/__init__.py", line 310, in get_prep_lookup
    return self.get_prep_value(value)
  File "django/db/models/fields/__init__.py", 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 by Luke Plant, 12 years ago

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 by fabiosantosart@…, 12 years ago

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 by dougvanhorn, 12 years ago

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:

try:
    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 12 years ago by dougvanhorn (previous) (diff)

comment:4 by dougvanhorn, 12 years ago

Cc: dougvanhorn added

comment:5 by Jonas H., 12 years ago

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 by Gerben Morsink, 9 years ago

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 by Ben Youngblut, 7 years ago

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

Action.objects.create(
    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