⚡️(backend) stop using LEFT(value, LENGTH(path)) in sql queries

Comparing path with LEFT(value, LENGTH(path)) makes a sequential scan on
all the Document table, the more this table grow, the more the query
using it will be slow. We dediced instead to lookup on the path
extracting all ancestors path for a given document and then make a path
IN statement to use the index existing on the path column.

CHANGELOG.md

@@ -13,6 +13,7 @@ and this project adheres to
 ### Changed
 
 - ⬆️(backend) upgrade celery to version 5.6.3
+- ⚡️(backend) stop using LEFT(value, LENGTH(path)) in sql queries
 
 ### Fixed
 

src/backend/core/api/viewsets.py

@@ -23,7 +23,7 @@ from django.core.validators import URLValidator
 from django.db import DatabaseError, connection, transaction
 from django.db import models as db
 from django.db.models.expressions import RawSQL
-from django.db.models.functions import Greatest, Left, Length
+from django.db.models.functions import Greatest
 from django.http import Http404, StreamingHttpResponse
 from django.urls import reverse
 from django.utils import timezone
@@ -1752,7 +1752,7 @@ class DocumentViewSet(
         # document. Filter to get the minimum access date for the logged-in user
         access_queryset = models.DocumentAccess.objects.filter(
             db.Q(user=user) | db.Q(team__in=user.teams),
-            document__path=Left(db.Value(document.path), Length("document__path")),
+            document__path__in=document.get_self_and_ancestors_paths(),
         ).aggregate(min_date=db.Min("created_at"))
 
         # Handle the case where the user has no accesses
@@ -1792,7 +1792,7 @@ class DocumentViewSet(
             access.created_at
             for access in models.DocumentAccess.objects.filter(
                 db.Q(user=user) | db.Q(team__in=user.teams),
-                document__path=Left(db.Value(document.path), Length("document__path")),
+                document__path__in=document.get_self_and_ancestors_paths(),
             )
         )
 

src/backend/core/models.py

@@ -1158,6 +1158,20 @@ class Document(MP_Node, BaseModel):
             Bucket=default_storage.bucket_name, Key=self.file_key, VersionId=version_id
         )
 
+    def get_self_and_ancestors_paths(self):
+        """
+        Return the paths of the document and of all its ancestors, computed from
+        the materialized path without querying the database.
+
+        Filtering on `path__in` with this list hits the unique index on `path`,
+        whereas comparing `path` with `LEFT(value, LENGTH(path))` forces a
+        sequential scan of the whole table.
+        """
+        return [
+            self.path[:pos]
+            for pos in range(self.steplen, len(self.path) + 1, self.steplen)
+        ]
+
     def get_nb_accesses_cache_key(self):
         """Generate a unique cache key for each document."""
         return f"document_{self.id!s}_nb_accesses"
@@ -1175,9 +1189,7 @@ class Document(MP_Node, BaseModel):
             nb_accesses = (
                 DocumentAccess.objects.filter(document=self).count(),
                 DocumentAccess.objects.filter(
-                    document__path=Left(
-                        models.Value(self.path), Length("document__path")
-                    ),
+                    document__path__in=self.get_self_and_ancestors_paths(),
                     document__ancestors_deleted_at__isnull=True,
                 ).count(),
             )
@@ -1217,7 +1229,7 @@ class Document(MP_Node, BaseModel):
         except AttributeError:
             roles = DocumentAccess.objects.filter(
                 models.Q(user=user) | models.Q(team__in=user.teams),
-                document__path=Left(models.Value(self.path), Length("document__path")),
+                document__path__in=self.get_self_and_ancestors_paths(),
             ).values_list("role", flat=True)
 
         return RoleChoices.max(*roles)

src/backend/core/tests/test_models_documents.py

@@ -1729,3 +1729,59 @@ def test_models_documents_compute_ancestors_links_paths_mapping_structure(
                 {"link_reach": sibling.link_reach, "link_role": sibling.link_role},
             ],
         }
+
+
+def test_models_documents_get_self_and_ancestors_paths_root():
+    """A root document should only return its own path."""
+    document = factories.DocumentFactory()
+
+    assert len(document.path) == models.Document.steplen
+    assert document.get_self_and_ancestors_paths() == [document.path]
+
+
+def test_models_documents_get_self_and_ancestors_paths_tree(
+    django_assert_num_queries,
+):
+    """
+    The method should return the paths of the document and all its ancestors,
+    ordered from the root down to the document itself, without hitting the database.
+    """
+    root = factories.DocumentFactory()
+    factories.DocumentFactory(parent=root)  # sibling branch, should be ignored
+    parent = factories.DocumentFactory(parent=root)
+    document = factories.DocumentFactory(parent=parent)
+    child = factories.DocumentFactory(parent=document)
+
+    with django_assert_num_queries(0):
+        paths = child.get_self_and_ancestors_paths()
+
+    assert paths == [root.path, parent.path, document.path, child.path]
+
+    # Should match what treebeard computes with a database query
+    ancestors_paths = list(
+        child.get_ancestors().order_by("path").values_list("path", flat=True)
+    )
+    assert paths == ancestors_paths + [child.path]
+
+    # Filtering on these paths should return exactly the ancestors and the document
+    assert set(
+        models.Document.objects.filter(path__in=paths).values_list("id", flat=True)
+    ) == {root.id, parent.id, document.id, child.id}
+
+
+def test_models_documents_get_self_and_ancestors_paths_from_path_only():
+    """
+    The method should only rely on the materialized path and the step length,
+    so it can be used on an unsaved instance.
+    """
+    steplen = models.Document.steplen
+    document = models.Document(path="0000001" + "000000A" + "00000Zz")
+
+    assert document.get_self_and_ancestors_paths() == [
+        "0000001",
+        "0000001000000A",
+        "0000001000000A00000Zz",
+    ]
+    assert all(
+        len(path) % steplen == 0 for path in document.get_self_and_ancestors_paths()
+    )