Add index on the last element of a path

arkindex/documents/api/elements.py

@@ -816,32 +816,22 @@ class ElementChildren(ElementsListBase):
     Requires a **read** access to the element's corpus.
     """
 
+    @property
+    def is_recursive(self):
+        recursive_param = self.clean_params.get('recursive')
+        return recursive_param is not None and recursive_param.lower() not in ('false', '0')
+
     @property
     def selected_corpus(self):
         return self.element.corpus
 
     def get_filters(self):
         filters = super().get_filters()
-        recursive_param = self.clean_params.get('recursive')
 
-        if recursive_param is None or recursive_param.lower() in ('false', '0'):
-            # Only list direct children, by listing all the direct paths from the current element
-            # This is faster for direct children than :
-            # - Element.objects.filter(paths__path__last=XXX)
-            # - Element.objects.get_descending()
-            # This code path is extremely used, as it's the basis for navigation amongst elements
-            # Be extremely careful when changing this behaviour.
-            paths = self.element.paths.values_list('path', flat=True)
-            if not paths:
-                # Support top level children
-                paths = [[]]
-
-            filters['paths__path__in'] = [
-                path + [self.kwargs['pk']]
-                for path in paths
-            ]
-        else:
+        if self.is_recursive:
             filters['paths__path__overlap'] = [self.kwargs['pk']]
+        else:
+            filters['paths__path__last'] = self.kwargs['pk']
 
         return filters
 

arkindex/documents/migrations/0047_elementpath_element_path_last.py

+# Generated by Django 4.0.1 on 2022-01-05 16:29
+
+import django.db.models.expressions
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('documents', '0046_transcription_orientation'),
+    ]
+
+    operations = [
+        migrations.AddIndex(
+            model_name='elementpath',
+            index=models.Index(django.db.models.expressions.F('path__last'), name='element_path_last'),
+        ),
+        # After this index is created, we need to analyze the table again to make its query planner see it;
+        # otherwise, it will keep its old way of querying and no performance improvement is made.
+        migrations.RunSQL(
+            "ANALYZE documents_elementpath",
+            reverse_sql=migrations.RunSQL.noop,
+        )
+    ]

arkindex/documents/models.py

@@ -121,6 +121,7 @@ class ElementPath(models.Model):
     class Meta:
         indexes = [
             GinIndex(fields=['path']),
+            models.Index('path__last', name='element_path_last'),
         ]
         constraints = [
             models.UniqueConstraint(fields=('element', 'path'), name='unique_element_paths'),

arkindex/documents/tests/test_children_elements.py

@@ -248,7 +248,7 @@ class TestChildrenElements(FixtureAPITestCase):
     def test_element_children_worker_version(self):
         self.corpus.elements.filter(name__contains='page 1r').update(worker_version=self.worker_version)
 
-        with self.assertNumQueries(7):
+        with self.assertNumQueries(6):
             response = self.client.get(
                 reverse('api:elements-children', kwargs={'pk': str(self.vol.id)}),
                 data={'worker_version': str(self.worker_version.id)}
@@ -307,7 +307,7 @@ class TestChildrenElements(FixtureAPITestCase):
         )
         element.add_parent(self.vol)
 
-        with self.assertNumQueries(8):
+        with self.assertNumQueries(7):
             response = self.client.get(
                 reverse('api:elements-children', kwargs={'pk': str(self.vol.id)}),
                 data={'with_has_children': True},
@@ -329,7 +329,7 @@ class TestChildrenElements(FixtureAPITestCase):
         )
 
     def test_children_modified_since_bad_format(self):
-        with self.assertNumQueries(2):
+        with self.assertNumQueries(1):
             response = self.client.get(
                 reverse('api:elements-children', kwargs={'pk': str(self.vol.id)}),
                 HTTP_IF_MODIFIED_SINCE='AAAAAAAAAAAAAAAAA',
@@ -338,7 +338,7 @@ class TestChildrenElements(FixtureAPITestCase):
         self.assertDictEqual(response.json(), {'If-Modified-Since': ['Bad date format']})
 
     def test_children_modified_since_not_modified(self):
-        with self.assertNumQueries(3):
+        with self.assertNumQueries(2):
             response = self.client.get(
                 reverse('api:elements-children', kwargs={'pk': str(self.vol.id)}),
                 HTTP_IF_MODIFIED_SINCE='Thu, 02 Apr 2099 13:37:42 GMT',
@@ -347,7 +347,7 @@ class TestChildrenElements(FixtureAPITestCase):
 
     def test_children_modified_since(self):
         self.corpus.elements.filter(name='Volume 1, page 1r').update(updated='2099-04-02T13:37:43Z')
-        with self.assertNumQueries(7):
+        with self.assertNumQueries(6):
             response = self.client.get(
                 reverse('api:elements-children', kwargs={'pk': str(self.vol.id)}),
                 HTTP_IF_MODIFIED_SINCE='Thu, 02 Apr 2099 13:37:42 GMT',
@@ -361,7 +361,7 @@ class TestChildrenElements(FixtureAPITestCase):
         )
 
     def test_children_filter_metadata_name(self):
-        with self.assertNumQueries(6):
+        with self.assertNumQueries(5):
             response = self.client.get(
                 reverse('api:elements-children', kwargs={'pk': str(self.vol.id)}),
                 data={'metadata_name': 'folio'}
@@ -388,7 +388,7 @@ class TestChildrenElements(FixtureAPITestCase):
         })
 
     def test_children_filter_metadata_name_value(self):
-        with self.assertNumQueries(6):
+        with self.assertNumQueries(5):
             response = self.client.get(
                 reverse('api:elements-children', kwargs={'pk': str(self.vol.id)}),
                 data={'metadata_name': 'folio', 'metadata_value': '1v'}
@@ -464,7 +464,7 @@ class TestChildrenElements(FixtureAPITestCase):
 
         for operator, value, expected_elements in parameters:
             with self.subTest(operator=operator, value=value):
-                with self.assertNumQueries(5):
+                with self.assertNumQueries(4):
                     response = self.client.get(
                         reverse('api:elements-children', kwargs={'pk': str(self.vol.id)}),
                         data={
@@ -480,7 +480,7 @@ class TestChildrenElements(FixtureAPITestCase):
                 )
 
     def test_children_filter_metadata_contains(self):
-        with self.assertNumQueries(6):
+        with self.assertNumQueries(5):
             response = self.client.get(
                 reverse('api:elements-children', kwargs={'pk': str(self.vol.id)}),
                 data={
@@ -513,8 +513,8 @@ class TestChildrenElements(FixtureAPITestCase):
         ]
         for rotation_angle, expected_elements in cases:
             with self.subTest(rotation_angle=rotation_angle):
-                # Requests that return no elements only make 3 SQL queries
-                num_queries = 5 if len(expected_elements) else 3
+                # Requests that return no elements only make 2 SQL queries
+                num_queries = 4 if len(expected_elements) else 2
                 with self.assertNumQueries(num_queries):
                     response = self.client.get(
                         reverse('api:elements-children', kwargs={'pk': str(self.vol.id)}),
@@ -564,7 +564,7 @@ class TestChildrenElements(FixtureAPITestCase):
         ]
         for mirrored, expected_elements in cases:
             with self.subTest(mirrored=mirrored):
-                with self.assertNumQueries(5):
+                with self.assertNumQueries(4):
                     response = self.client.get(
                         reverse('api:elements-children', kwargs={'pk': str(self.vol.id)}),
                         data={

arkindex/documents/tests/test_classes.py

@@ -330,7 +330,7 @@ class TestClasses(FixtureAPITestCase):
             )
 
     def test_element_children_classes(self):
-        with self.assertNumQueries(7):
+        with self.assertNumQueries(6):
             response = self.client.get(
                 reverse('api:elements-children', kwargs={'pk': str(self.parent.id)}),
                 data={'type': self.classified.slug, 'with_classes': 'yes'}

arkindex/documents/tests/test_destroy_elements.py

@@ -340,7 +340,7 @@ class TestDestroyElements(FixtureAPITestCase):
         self.client.force_login(self.user)
         element = self.corpus.elements.create(type=self.volume_type, name='Lonely element')
 
-        with self.assertNumQueries(8):
+        with self.assertNumQueries(7):
             response = self.client.delete(reverse('api:elements-children', kwargs={'pk': element.id}))
             self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
 
@@ -349,7 +349,7 @@ class TestDestroyElements(FixtureAPITestCase):
     @patch('arkindex.project.triggers.documents_tasks.element_trash.delay')
     def test_destroy_element_children(self, delay_mock):
         self.client.force_login(self.user)
-        with self.assertNumQueries(8):
+        with self.assertNumQueries(7):
             response = self.client.delete(reverse('api:elements-children', kwargs={'pk': self.vol.id}))
             self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)
 
@@ -370,7 +370,7 @@ class TestDestroyElements(FixtureAPITestCase):
     @patch('arkindex.project.triggers.documents_tasks.element_trash.delay')
     def test_destroy_element_children_delete_children(self, delay_mock):
         self.client.force_login(self.user)
-        with self.assertNumQueries(8):
+        with self.assertNumQueries(7):
             response = self.client.delete(
                 reverse('api:elements-children', kwargs={'pk': self.vol.id}),
                 QUERY_STRING='delete_children=false'