Support application of select_related and prefetch_related lookups to subqueries made by SpecificIterable

- Add queryset methods to reference docs, and provide performance considerations for prefetch_related()
pull/12472/head
Andy Babic 2024-09-19 17:49:19 +01:00 zatwierdzone przez LB (Ben Johnston)
rodzic fde2e6f26a
commit e451bbd96a
6 zmienionych plików z 253 dodań i 4 usunięć

Wyświetl plik

@ -4,6 +4,7 @@ Changelog
6.4 (xx.xx.xxxx) - IN DEVELOPMENT
~~~~~~~~~~~~~~~~
* Add the ability to apply basic Page QuerySet optimizations to `specific()` sub-queries using `select_related` & `prefetch_related` (Andy Babic)
* Fix: Improve handling of translations for bulk page action confirmation messages (Matt Westcott)
* Fix: Ensure custom rich text feature icons are correctly handled when provided as a list of SVG paths (Temidayo Azeez, Joel William, LB (Ben) Johnston)
* Fix: Ensure manual edits to `StreamField` values do not throw an error (Stefan Hammer)

Wyświetl plik

@ -282,4 +282,25 @@ menu_items = homepage.get_children().live().in_menu()
homepage.get_children().defer_streamfields().specific()
.. automethod:: first_common_ancestor
.. automethod:: select_related
.. automethod:: prefetch_related
#### Performance considerations
Typical usage of `prefetch_related()` results in an additional database query
being executed for each of the provided `lookups`. However, when combined with
`for_specific_subqueries=True`, this additional number of database queries is
multiplied for each specific type in the result. If you are only fetching
a small number of objects, or the type-variance of results is likely to be high,
the additional overhead of making these additional queries could actually have a
negative impact on performance.
Using `prefetch_related()` with `for_specific_subqueries=True` should be reserved
for cases where a large number of results is needed, or the type-variance is
retricted in some way. For example, when rendering a list of child pages where
`allow_subtypes` is set on the parent, limiting the results to a small number of
page types. Or, where the `type()` or `not_type()` filters have been applied to
restrict the queryset to a small number of specific types.
```

Wyświetl plik

@ -14,7 +14,7 @@ depth: 1
### Other features
* ...
* Add the ability to apply basic Page QuerySet optimizations to `specific()` sub-queries using `select_related` & `prefetch_related`, see [](../reference/pages/queryset_reference.md) (Andy Babic)
### Bug fixes

Wyświetl plik

@ -147,11 +147,17 @@ class SpecificQuerySetMixin:
super().__init__(*args, **kwargs)
# set by PageQuerySet.defer_streamfields()
self._defer_streamfields = False
self._specific_select_related_fields = ()
self._specific_prefetch_related_lookups = ()
def _clone(self):
"""Ensure clones inherit custom attribute values."""
clone = super()._clone()
clone._defer_streamfields = self._defer_streamfields
clone._specific_select_related_fields = self._specific_select_related_fields
clone._specific_prefetch_related_lookups = (
self._specific_prefetch_related_lookups
)
return clone
def specific(self, defer=False):
@ -179,6 +185,137 @@ class SpecificQuerySetMixin:
(SpecificIterable, DeferredSpecificIterable),
)
def select_related(self, *fields, for_specific_subqueries: bool = False):
"""
Overrides Django's native :meth:`~django.db.models.query.QuerySet.select_related`
to allow related objects to be fetched by the subqueries made when a specific
queryset is evaluated.
When ``for_specific_subqueries`` is ``False`` (the default), the method functions
exactly like the original method. However, when ``True``, ``fields`` are
**required**, and must match names of ForeignKey fields on all specific models
that might be included in the result (which can include fields inherited from
concrete parents). Unlike when ``for_specific_subqueries`` is ``False``, no
validation is applied to ``fields`` when the method is called. Rather, that when
the method is called. Instead, that validation is applied for each individual
subquery when the queryset is evaluated. This difference in behaviour should be
taken into account when experimenting with ``for_specific_subqueries=True`` .
As with Django's native implementation, you chain multiple applications of
``select_related()`` with ``for_specific_subqueries=True`` to progressively add
to the list of fields to be fetched. For example:
.. code-block:: python
# Fetch 'author' when retrieving specific page data
queryset = Page.objects.specific().select_related("author", for_specific_subqueries=True)
# We're rendering cards with images, so fetch the listing image too
queryset = queryset.select_related("listing_image", for_specific_subqueries=True)
# Fetch some key taxonomy data too
queryset = queryset.select_related("topic", "target_audience", for_specific_subqueries=True)
As with Django's native implementation, ``None`` can be supplied in place of
``fields`` to negate a previous application of ``select_related()``. By default,
this will only work for cases where ``select_related()`` was called without
``for_specific_subqueries``, or with ``for_specific_subqueries=False``. However,
you can use ``for_specific_subqueries=True`` to negate subquery-specific
applications too. For example:
.. code-block:: python
# Fetch 'author' and 'listing_image' when retrieving specific page data
queryset = Page.objects.specific().select_related(
"author",
"listing_image",
for_specific_subqueries=True
)
# I've changed my mind. Do not fetch any additional data
queryset = queryset.select_related(None, for_specific_subqueries=True)
"""
if not for_specific_subqueries:
return super().select_related(*fields)
if not fields:
raise ValueError(
"'fields' must be specified when calling select_related() with for_specific_subqueries=True"
)
clone = self._chain()
if fields == (None,):
clone._specific_select_related_fields = ()
else:
clone._specific_select_related_fields = (
self._specific_select_related_fields + fields
)
return clone
def prefetch_related(self, *lookups, for_specific_subqueries: bool = False):
"""
Overrides Django's native :meth:`~django.db.models.query.QuerySet.prefetch_related`
implementation to allow related objects to be fetched alongside the subqueries made
when a specific queryset is evaluated.
When ``for_specific_subqueries`` is ``False`` (the default), the method functions
exactly like the original method. However, when ``True``, ``lookups`` are
**required**, and must match names of related fields on all specific models that
might be included in the result (which can include relationships inherited from
concrete parents). Unlike when ``for_specific_subqueries`` is ``False``, no
validation is applied to ``lookups`` when the method is called. Instead, that
validation is applied for each individual subquery when the queryset is
evaluated. This difference in behaviour should be taken into account when
experimenting with ``for_specific_subqueries=True``.
As with Django's native implementation, you chain multiple applications of
``prefetch_related()`` with ``for_specific_subqueries=True`` to progressively
add to the list of lookups to be made. For example:
.. code-block:: python
# Fetch 'contributors' when retrieving specific page data
queryset = Page.objects.specific().prefetch_related("contributors", for_specific_subqueries=True)
# We're rendering cards with images, so prefetch listing image renditions too
queryset = queryset.prefetch_related("listing_image__renditions", for_specific_subqueries=True)
# Fetch some key taxonomy data also
queryset = queryset.prefetch_related("tags", for_specific_subqueries=True)
As with Django's native implementation, ``None`` can be supplied in place of
``lookups`` to negate a previous application of ``prefetch_related()``. By default,
this will only work for cases where ``prefetch_related()`` was called without
``for_specific_subqueries``, or with ``for_specific_subqueries=False``. However,
you can use ``for_specific_subqueries=True`` to negate subquery-specific
applications too. For example:
.. code-block:: python
# Fetch 'contributors' and 'listing_image' renditions when retrieving specific page data
queryset = Page.objects.specific().prefetch_related(
"contributors",
"listing_image__renditions",
for_specific_subqueries=True
)
# I've changed my mind. Do not make any additional queries
queryset = queryset.prefetch_related(None, for_specific_subqueries=True)
"""
if not for_specific_subqueries:
return super().prefetch_related(*lookups)
if not lookups:
raise ValueError(
"'lookups' must be provided when calling prefetch_related() with for_specific_subqueries=True"
)
clone = self._chain()
if lookups == (None,):
clone._specific_prefetch_related_lookups = ()
else:
clone._specific_prefetch_related_lookups = (
self._specific_prefetch_related_lookups + lookups
)
return clone
class PageQuerySet(SearchableQuerySetMixin, SpecificQuerySetMixin, TreeQuerySet):
def live_q(self):
@ -561,6 +698,14 @@ class SpecificIterable(ModelIterable):
model = content_types[content_type].model_class() or qs.model
items = model.objects.filter(pk__in=pks)
if qs._specific_select_related_fields:
items = items.select_related(*qs._specific_select_related_fields)
if qs._specific_prefetch_related_lookups:
items = items.prefetch_related(
*qs._specific_prefetch_related_lookups
)
if qs._defer_streamfields and hasattr(items, "defer_streamfields"):
items = items.defer_streamfields()

Wyświetl plik

@ -141,7 +141,8 @@
"audience": "public",
"location": "The moon",
"body": "<p>I haven't worked out the details yet, but it's going to have cake and ponies</p>",
"cost": "Free"
"cost": "Free",
"feed_image": 1
}
},
@ -170,7 +171,8 @@
"audience": "private",
"location": "The moon",
"body": "<p>your name's not down, you're not coming in</p>",
"cost": "Free (but not for you)"
"cost": "Free (but not for you)",
"feed_image": 1
}
},
@ -246,7 +248,8 @@
"audience": "public",
"location": "Hobart",
"body": "<p>Party time</p>",
"cost": "free"
"cost": "free",
"feed_image": 1
}
},

Wyświetl plik

@ -919,6 +919,85 @@ class TestSpecificQuery(WagtailTestUtils, TestCase):
self.assertEqual(results.first().subscribers_count, 1)
self.assertEqual(results.last().subscribers_count, 1)
def test_specific_subquery_select_related(self):
with self.assertNumQueries(2):
pages = list(
Page.objects.type(EventPage)
.specific()
.select_related("feed_image", for_specific_subqueries=True)
)
self.assertEqual(len(pages), 4)
with self.assertNumQueries(0):
for page in pages:
self.assertTrue(page.feed_image)
def test_specific_subquery_select_related_without_fields(
self,
):
with self.assertRaises(ValueError):
Page.objects.all().select_related(for_specific_subqueries=True)
def test_specific_subquery_select_related_negation(self):
with self.assertNumQueries(2):
pages = list(
Page.objects.type(EventPage)
.specific()
.select_related("feed_image", for_specific_subqueries=True)
.select_related(
None, for_specific_subqueries=True
) # This should negate the above line
)
with self.assertNumQueries(4):
for page in pages:
self.assertTrue(page.feed_image)
def test_specific_subquery_prefetch_related(self):
with self.assertNumQueries(3):
pages = list(
Page.objects.type(EventPage)
.specific()
.prefetch_related("categories", for_specific_subqueries=True)
)
self.assertEqual(len(pages), 4)
with self.assertNumQueries(0):
for page in pages:
self.assertFalse(page.categories.all())
def test_specific_subquery_prefetch_related_without_lookups(self):
with self.assertRaises(ValueError):
Page.objects.all().prefetch_related(for_specific_subqueries=True)
def test_specific_subquery_prefetch_related_negation(self):
with self.assertNumQueries(2):
pages = list(
Page.objects.type(EventPage)
.specific()
.prefetch_related("categories", for_specific_subqueries=True)
.prefetch_related(
None, for_specific_subqueries=True
) # This should negate the above line
)
self.assertEqual(len(pages), 4)
with self.assertNumQueries(4):
for page in pages:
self.assertFalse(page.categories.all())
def test_specific_subquery_select_related_and_prefetch_related(self):
with self.assertNumQueries(3):
pages = list(
Page.objects.type(EventPage)
.specific()
.select_related("feed_image", for_specific_subqueries=True)
.prefetch_related(
"feed_image__renditions", for_specific_subqueries=True
)
)
self.assertEqual(len(pages), 4)
with self.assertNumQueries(0):
for page in pages:
self.assertTrue(page.feed_image)
self.assertFalse(page.feed_image.renditions.all())
def test_specific_query_with_alias(self):
"""
Ensure alias() works with specific() queries.