Django internals call len(queryset) instead of queryset.count()

[Chris Wilson]

The documentation says:

Note: Don’t use len() on QuerySets if all you want to do is determine the number of records in the set. It’s much more efficient to handle a count at the database level, using SQL’s SELECT COUNT(*), and Django provides a count() method for precisely this reason. See count() below.

But Django does this itself in a few places:

$ grep -r 'len.*queryset' django/
django/forms/models.py:            return len(self.get_queryset())
django/forms/models.py:        return (len(self.queryset) +
django/contrib/admin/actions.py:    if len(queryset) == 1:

[Chris Wilson]

Unfortunately it seems that calling count() is sometimes more expensive than len(queryset). When I made this change, admin_views.tests.AdminCustomQuerysetTest.test_changelist_view_count_queries started failing because the number of queries increased from 4 to 15!

So is this a wontfix, or can we improve the efficiency of count()?

​https://github.com/aptivate/django/tree/ticket_22209

[Anssi Kääriäinen]

Calling len is cheap if the queryset is already executed (or it is going to be executed again later on). Adding comments in the places where this is the case and fixing other places is the right fix for this.

[Chris Wilson]

@akaariai how about we make count() check whether the queryset is already executed, and return its length if it is? Then all Django users will get the benefit without worrying about tricky cases like different code paths and passed-in querysets.

[Anssi Kääriäinen]

Looking at the code this seems to be already the case. So, here using len will execute the query, and later on the results can be reused. Not sure if the results are actually reused in all cases.

Adding comments where results are reused and changing other places to use count seems still like thw best option.

Similar issue is using "if qs:" - this is very inefficient if the qs isn't reused. Not sure if django has any problems from this antipattern.

[Tim Graham]

Please don't close a ticket without comment.

[Collin Anderson]

Changing the behavior of count() to magically check the length of the cached data if it exists only helps if the queryset is evaluated first. It's still better to use len() rather than count() in this case:

if queryset.count():
    for x in queryset:
        # do something.
}}

I've actually gone through a lot of my code and changed count() to len() in a lot of cases it where it was actually making things slower.
It seems to me it's not worth changing anything here except for maybe changing the warning to be less severe.

[Tim Graham]

The places in the ticket description look okay to me to use len() instead of count().