Opened 7 years ago

Closed 5 years ago

Last modified 4 years ago

#7235 closed (fixed)

filtering on an EmptyQuerySet raises an Exception

Reported by: anonymous Owned by: aljosa
Component: Database layer (models, ORM) Version: master
Severity: Keywords: qsrf-cleanup query, emptyqueryset
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Trying to filter an instance of EmptyQuerySet causes an Exception to be raised:

>>> from django.db.models.query import *
>>> q = EmptyQuerySet()
>>> q.filter(x=10)
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/django/db/models/query.py", line 370, in filter
    return self._filter_or_exclude(False, *args, **kwargs)
  File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/django/db/models/query.py", line 388, in _filter_or_exclude
    clone.query.add_q(Q(*args, **kwargs))
  File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/django/db/models/sql/query.py", line 1068, in add_q
    can_reuse=used_aliases)
  File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/django/db/models/sql/query.py", line 954, in add_filter
    opts = self.get_meta()
  File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/site-packages/django/db/models/sql/query.py", line 127, in get_meta
    return self.model._meta
AttributeError: 'NoneType' object has no attribute '_meta'
>>> 

I would think that no matter what you pass in as arguments to filter(), it should always return EmptyQuerySet.

Attachments (2)

7235.diff (2.1 KB) - added by aljosa 6 years ago.
EmptyQuerySet methods all, filter, exclude, complex_filter, select_related, annotate, order_by, distinct, extra, reverse, defer and only "return self"
7235_tests.diff (1.3 KB) - added by taylormarshall 5 years ago.
regression tests for 7235

Download all attachments as: .zip

Change History (16)

comment:1 Changed 7 years ago by john_scott

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

I can confirm that the above error occurs with Django version 0.97-pre-SVN-7477

With Django version 0.97-pre-SVN-7476 I get the expected behavior:

>>> from django.db.models.query import EmptyQuerySet
>>> q = EmptyQuerySet()
>>> q.filter(x=10)
[]

comment:2 Changed 7 years ago by gav

  • Keywords qsrf-cleanup added

comment:3 Changed 7 years ago by emulbreh

  • Component changed from Core framework to Database wrapper
  • Triage Stage changed from Unreviewed to Design decision needed

Do QuerySets without an associated Model make sense?

Solution proposals:

# Keep EmptyQuerySet, require the model argument: nothing to do.
# Keep EmptyQuerySet, make the model argument optional: overwrite all methods that return a derived QuerySet and return self (since QuerySets are immutable). This would be uncomfortable to maintain.
# Remove EmptyQuerySet, add an is_empty flag to Query: given that Query is already 1.5kloc ... (allthough enabling something like query.mark_empty() in add_to_query() methods would be nice).

Additionally: If EmptyQuerySet doesn't require the model argument, there might as well be a single instance. Otherwise one instance per model would do.

I'd give this a try, as long as the tempting third option isn't choosen (I'm not comfortable with the Query code).

comment:4 Changed 7 years ago by jacob

  • milestone set to 1.0

comment:5 Changed 7 years ago by mtredinnick

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

This isn't a valid problem, since creating an EmptyQuerySet directly is not something you ever do. It's an instance that is created (correctly) using the none() method on other querysets or model managers. Internal bits of code can sometimes create QuerySet classes and subclasses without immediately setting the model, which is why it's permitted, but this bug report is just a case of "if you use the code incorrectly, you get nonsense results." There is nothing in the documentation to suggest that what you tried here should work, whereas working with the result of a none() call does (or should) work correctly.

comment:6 Changed 6 years ago by AdamG

I believe this should be reopened because of the AnonymousUser use case.

django.contrib.auth.models.AnonymousUser provides a
pseudo-related-manager under AnonymousUser.groups. This makes a call
like request.user.groups.filter(name__in=group_names) unsafe - if
you get an AnonymousUser object instead of a User, you'll get an
exception instead of what I think would be the correct behavior - an
EmptyQuerySet. It may be that django.contrib.auth is using
EmptyQuerySet incorrectly - as a public API instead as a temporary
pre-real-QuerySet as Malcom indicates. But, I don't think there's a
way to fix that in a backwards-compatible manner; after all,
AnonymousUser is not a model, and switching to a real QuerySet
is not an option.

The other alternative I can see is for programmers to either typecheck
request.user or check request.user.is_anonymous() before
attempting to filter on request.user.groups or
request.user.user_permissions. I don't think that this is either
correct or friendly.

comment:7 Changed 6 years ago by mtredinnick

  • Resolution invalid deleted
  • Status changed from closed to reopened
  • Triage Stage changed from Design decision needed to Accepted

In light of AdamG's observation, which I happened to notice going past (start a thread on django-dev next time, rather than commenting on a closed ticket, please), reopening. That's a valid case.

The fix is to make every queryset operation on EmptyQuerySet pretty much just return self.

comment:8 Changed 6 years ago by gwilson

  • milestone changed from 1.0 to 1.1

comment:9 Changed 6 years ago by aljosa

  • Owner changed from nobody to aljosa
  • Status changed from reopened to new

comment:10 Changed 6 years ago by jacob

  • milestone changed from 1.1 to 1.2

Punting from 1.1: this is annoying, but there's an easy (if ugly) workaround for now.

Changed 6 years ago by aljosa

EmptyQuerySet methods all, filter, exclude, complex_filter, select_related, annotate, order_by, distinct, extra, reverse, defer and only "return self"

comment:11 Changed 6 years ago by Travis Cline <travis.cline@…>

  • Has patch set
  • Needs tests set

Changed 5 years ago by taylormarshall

regression tests for 7235

comment:12 Changed 5 years ago by taylormarshall

  • Needs tests unset

comment:13 Changed 5 years ago by adrian

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

(In [12147]) Fixed #7235 -- EmptyQuerySet no longer raises and exception when it's filter()ed (along with some other QuerySet methods). Thanks, taylormarshall

comment:14 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