Skip to content

Commit 564506a

Browse files
authored
Avoid calling .exists twice in the same query (#637)
If the object to check was a queryset, exist would be called twice. Instead of dealing with checks in the template, I'm just using a filter. Tests on .org will need to be updated after this is merged to reduce the expected queries.
1 parent ccbc569 commit 564506a

File tree

2 files changed

+21
-11
lines changed

2 files changed

+21
-11
lines changed

readthedocsext/theme/templates/includes/crud/table_list.html

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
{% load is_empty from ext_theme_tags %}
12
{% comment "rst" %}
23
Shared list base
34
================
@@ -92,21 +93,16 @@
9293
{% comment %}
9394
We usually pass in a queryset here, but some views emit arrays of objects
9495
instead, so we support these as well. Therefore, the check here needs to be
95-
for an empty array or an empty queryset. Because Django templates aim to
96-
be as difficult as possible to use, this conditional looks pretty silly.
96+
for an empty array or an empty queryset.
9797

98-
This cannot be a simple `if not objects` as this executes the query, which
99-
can cause the view to timeout when the query is very large. See ``bool``
100-
evaluation in:
98+
We avoid using a simple `if not objects` check, as that would evaluate
99+
the queryset, which can cause a timeout if the queryset is very large,
100+
instead we use a custom template tag that call ``exists`` if the value is
101+
a queryset, or ``not value`` otherwise. See ``bool`` evaluation in:
101102

102103
https://docs.djangoproject.com/en/4.2/ref/models/querysets/
103-
104-
Order on the conditional matters here, `and` has higher precedence. Django
105-
templates don't support parenthesis in conditionals.
106-
107-
https://docs.djangoproject.com/en/5.0/ref/templates/builtins/#boolean-operators
108104
{% endcomment %}
109-
{% if objects.exists is None and objects|length == 0 or objects.exists == False %}
105+
{% if objects|is_empty %}
110106
{% comment "rst" %}
111107

112108
.. _api-template-list-placeholder:

readthedocsext/theme/templatetags/ext_theme_tags.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import logging
22
from urllib.parse import urljoin
33

4+
from django.db.models.query import QuerySet
5+
46
from django import template
57
from django.conf import settings
68
from django.templatetags.i18n import (
@@ -211,3 +213,15 @@ def readthedocs_language_name_local(lang_code):
211213
except Exception:
212214
log.exception("Error getting language name")
213215
return lang_code
216+
217+
218+
@register.filter
219+
def is_empty(value):
220+
"""
221+
Check if an iterable or queryset is empty.
222+
223+
This avoids using `not value` on a queryset, so the queryset is not evaluated.
224+
"""
225+
if isinstance(value, QuerySet):
226+
return not value.exists()
227+
return not value

0 commit comments

Comments
 (0)