#19261: Page.__getitem__ usage causes queryset to evaluate when doing method
lookups
-------------------------+-------------------------------------------------
Reporter: trbs | Owner: nobody
Type: Bug | Status: new
Component: Core | Version: 1.4
(Other) | Keywords: page, type_error, __getitem__,
Severity: Normal | pagination
Triage Stage: | Has patch: 1
Unreviewed | UI/UX: 0
Easy pickings: 1 |
-------------------------+-------------------------------------------------
The code for Page.__getitem__ forces an evaluation (query to the database)
of the queryset when it's actually looking up the name the name of class
functions.
Python: 2.7 (think 2.6 as well)
Django: 1.4.x
The function looks like this:
{{{
def __getitem__(self, index):
# The object_list is converted to a list so that if it was a
QuerySe t
# it won't be a database hit per __getitem__.
return super(Page, self).__getitem__(self, index)
}}}
Now consider this template code that runs from a generic class based view:
{{{
{% if page_obj.has_previous %}
...
{% endif %}
}}}
Here ```page_obj.has_previous``` generates a call to __getitem__ with
'has_previous' as its parameter ```index```.
This translates to:
{{{
return list(self.object_list)['has_previous']
}}}
Which raises a ```TypeError```: ```list indices must be integers, not
unicode````.
And as per
http://docs.python.org/2/reference/datamodel.html#object.__getitem__ this
is nicely ignored and __getitem__ returns the correct attribute.
However before the type error, during list(self.object_list) the queryset
is executed. This should not happen if it's not necessary. It defeats
caching in some cases, the query could be expensive and is generally not
what we would expect when calling a Page instance's methods.
To avoid this for the (quite common) usage of __getitem__, I suggest
something in the lines of:
{{{
def __getitem__(self, index):
# The object_list is converted to a list so that if it was a
QuerySet
# it won't be a database hit per __getitem__.
if isinstance(index, (int,long)):
return list(self.object_list)[index]
return super(Page, self).__getitem__(self, index)
}}}
(Maybe we don't need to check for long ? and is there maybe some other
type that we should check ?)
Patch does not yet includes tests for this. However I'm not really sure
what test would actually check this. As the type error is actually
swallowed by Python itself.
(btw: i set easy-pickings, as finding it out was not necessarily 'easy'
but fixing it should hopefully be)
--
Ticket URL: <https://code.djangoproject.com/ticket/19261>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
