#19653 closed Cleanup/optimization (fixed)
`Manager.get_empty_query_set` should call `get_query_set`
| Reported by: | Simon Charette | Owned by: | nobody |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
Now that EmptyQuerySet has been removed calling none() on a QuerySet subclass returns an instance that subclass:
>>> class MyQuerySet(models.query.QuerySet): pass >>> isinstance(MyQuerySet(MyModel).none(), MyQuerySet) >>> True # On django >= 1.6
This is quite useful and less error prone.
However, when one subclass models.Manager to return a QuerySet subclass they must override both get_empty_query_set and get_query_set to have consistent behavior.
class MyManager(models.Manager):
def get_query_set(self):
return MyQuerySet(self.model, using=self._db)
class MyModel(models.Model):
objects = MyManager()
>>> isinstance(MyModel.objects.none(), MyQuerySet) # Manager.none() calls get_empty_query_set under the hood
>>> False
>>> isinstance(MyModel.objects.all().none(), MyQuerySet)
>>> True
Another alternative (better IMHO) is to completely remove get_empty_query_set since it's not documented and only used by Manager.none and EmptyManager.get_query_set. We could make Manager.none proxy QuerySet.none and EmptyManager.get_query_set returns QuerySet.get_query_self(self).none().
Attaching two patches that both pass all tests on Python 2.7.3 SQlite.
Attachments (2)
Change History (7)
by , 13 years ago
| Attachment: | 0001-Fixed-19653-Make-sure-get_empty_query_set-calls-get_.patch added |
|---|
by , 13 years ago
| Attachment: | 0001-Fixed-19653-Removed-Manager.get_empty_query_set.patch added |
|---|
comment:1 by , 13 years ago
| Triage Stage: | Unreviewed → Ready for checkin |
|---|
comment:2 by , 13 years ago
Anybody know of any real use cases get_empty_query_set()? If not, I will go with the remove get_empty_query_set() approach.
I will commit this in a day or two if nobody objects...
comment:3 by , 13 years ago
Looks like get_empty_query_set was introduced at the same time as EmptyQuerySet.
comment:4 by , 13 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
comment:5 by , 13 years ago
| Component: | ORM aggregation → Database layer (models, ORM) |
|---|
Looks good to me. Marking as ready for checkin, I like the full removal approach more, it looks like it will not be needed any more.