mirror of
https://github.com/paperless-ngx/paperless-ngx.git
synced 2026-08-26 12:43:19 +00:00
perf: skip effective_content annotation on document list unless filtered on
DocumentViewSet.get_queryset() always attached a correlated subquery resolving each document's latest version content, even though it's only needed for the deprecated search/title_content/content__* filter params. Evaluated for every candidate row before pagination's LIMIT, this is pathological on MariaDB: its default cardinality estimate for the mostly- NULL root_document_id self-join drives it to a near-full-table scan per row instead of using the FK index, turning a normal filtered list request into a multi-second query (root cause of paperless-ngx#13778's report). Only attach the annotation when a request actually filters on it. The common case now relies on Document.get_effective_content()'s existing prefetch-based fallback instead (extended the "versions" prefetch to include content), which DocumentSerializer.to_representation() now calls directly instead of checking for the annotation via hasattr().
This commit is contained in:
@@ -1146,8 +1146,13 @@ class DocumentSerializer(
|
||||
|
||||
def to_representation(self, instance):
|
||||
doc = super().to_representation(instance)
|
||||
if "content" in self.fields and hasattr(instance, "effective_content"):
|
||||
doc["content"] = getattr(instance, "effective_content") or ""
|
||||
if "content" in self.fields:
|
||||
# get_effective_content() already falls back through the SQL
|
||||
# annotation (when the queryset attached one), the prefetched
|
||||
# "versions" cache, and finally a direct query -- so this stays
|
||||
# correct whether or not DocumentViewSet.get_queryset() decided
|
||||
# the annotation was needed for this request.
|
||||
doc["content"] = instance.get_effective_content() or ""
|
||||
if self.truncate_content and "content" in self.fields:
|
||||
doc["content"] = doc.get("content")[0:550]
|
||||
return doc
|
||||
|
||||
@@ -0,0 +1,91 @@
|
||||
from __future__ import annotations
|
||||
|
||||
from types import SimpleNamespace
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
import pytest
|
||||
from django.db import connection
|
||||
from django.test.utils import CaptureQueriesContext
|
||||
from rest_framework import status
|
||||
|
||||
from documents.tests.factories import DocumentFactory
|
||||
from documents.views import DocumentViewSet
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from rest_framework.test import APIClient
|
||||
|
||||
|
||||
class TestNeedsEffectiveContentAnnotation:
|
||||
"""
|
||||
DocumentViewSet._needs_effective_content_annotation() decides whether
|
||||
the effective_content correlated subquery is worth attaching to the
|
||||
queryset at all -- see TestDocumentListEffectiveContentAnnotation below
|
||||
for why. This only checks that decision's own logic (a plain query-param
|
||||
membership test), not that Django/DRF's filtering machinery works.
|
||||
"""
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("params", "expected"),
|
||||
[
|
||||
({}, False),
|
||||
({"ordering": "-added"}, False),
|
||||
({"tags__id__in": "1,2"}, False),
|
||||
({"search": "foo"}, True),
|
||||
({"title_content": "foo"}, True),
|
||||
({"content__istartswith": "foo"}, True),
|
||||
({"content__iendswith": "foo"}, True),
|
||||
({"content__icontains": "foo"}, True),
|
||||
({"content__iexact": "foo"}, True),
|
||||
],
|
||||
)
|
||||
def test_detects_content_filter_params(
|
||||
self,
|
||||
params: dict[str, str],
|
||||
expected: bool, # noqa: FBT001
|
||||
) -> None:
|
||||
# GIVEN a view bound to a request carrying the given query params
|
||||
view = DocumentViewSet()
|
||||
view.request = SimpleNamespace(query_params=params)
|
||||
|
||||
# WHEN checking whether the effective_content annotation is needed
|
||||
# THEN it's needed only for requests that actually filter on it
|
||||
assert view._needs_effective_content_annotation() is expected
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
class TestDocumentListEffectiveContentAnnotation:
|
||||
"""
|
||||
DocumentViewSet.get_queryset() only attaches the effective_content
|
||||
correlated subquery when a request actually filters on it. Attaching it
|
||||
unconditionally re-executes it once per candidate row before the page's
|
||||
LIMIT is applied -- fine on SQLite/Postgres, but pathological on
|
||||
MariaDB's default cardinality estimation for the root_document_id
|
||||
self-join once candidate counts get large (see the root_document_id /
|
||||
effective_content perf investigation).
|
||||
"""
|
||||
|
||||
def test_list_without_content_filter_skips_annotation_but_returns_latest_content(
|
||||
self,
|
||||
admin_client: APIClient,
|
||||
) -> None:
|
||||
# GIVEN a root document whose latest version has different content
|
||||
root = DocumentFactory(content="old-root-content")
|
||||
DocumentFactory(
|
||||
root_document=root,
|
||||
version_index=1,
|
||||
content="new-version-content",
|
||||
)
|
||||
|
||||
# WHEN listing documents with no search/content-filter param
|
||||
with CaptureQueriesContext(connection) as ctx:
|
||||
response = admin_client.get("/api/documents/?fields=id,content")
|
||||
|
||||
# THEN the response still reflects the latest version's content...
|
||||
assert response.status_code == status.HTTP_200_OK
|
||||
assert response.data["results"] == [
|
||||
{"id": root.id, "content": "new-version-content"},
|
||||
]
|
||||
# ...without the database ever evaluating effective_content per row
|
||||
assert not any(
|
||||
"effective_content" in query["sql"] for query in ctx.captured_queries
|
||||
)
|
||||
+38
-7
@@ -1085,12 +1085,33 @@ class DocumentViewSet(
|
||||
],
|
||||
}
|
||||
|
||||
# Query params whose filtering needs effective_content evaluated in SQL
|
||||
# against every candidate row -- see _needs_effective_content_annotation().
|
||||
_CONTENT_FILTER_PARAMS = (
|
||||
"search", # DRF SearchFilter's search_fields includes effective_content
|
||||
"title_content",
|
||||
"content__istartswith",
|
||||
"content__iendswith",
|
||||
"content__icontains",
|
||||
"content__iexact",
|
||||
)
|
||||
|
||||
def _needs_effective_content_annotation(self) -> bool:
|
||||
# effective_content is a per-row correlated subquery resolving each
|
||||
# document's latest version. Cheap when evaluated only for the page
|
||||
# that survives filtering/sorting/pagination (the common case, via
|
||||
# the "versions" prefetch + Document.get_effective_content()'s
|
||||
# fallback), but if anything filters *on* it, the database has to
|
||||
# evaluate it for every candidate row before the LIMIT is reached --
|
||||
# pathological on MariaDB specifically for the root_document_id
|
||||
# self-join once real candidate counts get large. Everything on this
|
||||
# list is deprecated in favor of the Tantivy-backed search endpoint
|
||||
# (see filters.py's TitleContentFilter/EffectiveContentFilter docs),
|
||||
# so keep paying that cost only when one is actually used.
|
||||
params = self.request.query_params
|
||||
return any(param in params for param in self._CONTENT_FILTER_PARAMS)
|
||||
|
||||
def get_queryset(self):
|
||||
latest_version_content = Subquery(
|
||||
versions_newest_first(
|
||||
Document.objects.filter(root_document=OuterRef("pk")),
|
||||
).values("content")[:1],
|
||||
)
|
||||
# A correlated subquery avoids the LEFT JOIN + Count() this used to
|
||||
# be, which forced a GROUP BY aggregate over every matching document
|
||||
# before the query could even be sorted or limited.
|
||||
@@ -1110,10 +1131,9 @@ class DocumentViewSet(
|
||||
# ObjectFilter.filter(). A blanket .distinct() here forces the
|
||||
# database to fully sort and dedupe every visible document before
|
||||
# it can apply LIMIT, which is disastrous at scale.
|
||||
return (
|
||||
queryset = (
|
||||
Document.objects.filter(root_document__isnull=True)
|
||||
.order_by("-created", "-id")
|
||||
.annotate(effective_content=Coalesce(latest_version_content, F("content")))
|
||||
.annotate(num_notes=Coalesce(note_count, 0))
|
||||
.select_related("correspondent", "storage_path", "document_type", "owner")
|
||||
.prefetch_related(
|
||||
@@ -1126,6 +1146,7 @@ class DocumentViewSet(
|
||||
"version_label",
|
||||
"root_document_id",
|
||||
"version_index",
|
||||
"content",
|
||||
),
|
||||
),
|
||||
"tags",
|
||||
@@ -1136,6 +1157,16 @@ class DocumentViewSet(
|
||||
"notes",
|
||||
)
|
||||
)
|
||||
if self._needs_effective_content_annotation():
|
||||
latest_version_content = Subquery(
|
||||
versions_newest_first(
|
||||
Document.objects.filter(root_document=OuterRef("pk")),
|
||||
).values("content")[:1],
|
||||
)
|
||||
queryset = queryset.annotate(
|
||||
effective_content=Coalesce(latest_version_content, F("content")),
|
||||
)
|
||||
return queryset
|
||||
|
||||
def get_serializer(self, *args, **kwargs):
|
||||
fields_param = self.request.query_params.get("fields", None)
|
||||
|
||||
Reference in New Issue
Block a user