Make the query list scale with query history; fix N+1 on the log page

HISTORY.rst

@@ -7,6 +7,10 @@ This project adheres to `Semantic Versioning <https://semver.org/>`_.
 
 Unreleased
 ===========================
+* `#722`_: The query list (home page) no longer loads the entire query log history on every view. Run counts and
+  last runs are computed in SQL, so the page stays fast as logs accumulate (300k logs: ~9.7s -> ~0.9s). The query
+  list and query log pages also no longer run an extra database query per row.
+
 * `#713`_: Connections whose database driver isn't installed (e.g. MySQL without ``mysqlclient``) can no longer be
   saved, and are marked "(driver not installed)" in the engine dropdown. Existing ones, and unreachable databases, now
   show a readable error in the schema pane, Test connection, and query runs instead of a 500.
@@ -709,6 +713,7 @@ Initial Release
 .. _#664: https://github.com/explorerhq/sql-explorer/pull/664
 .. _#687: https://github.com/explorerhq/sql-explorer/pull/687
 .. _#703: https://github.com/explorerhq/sql-explorer/pull/703
+.. _#722: https://github.com/explorerhq/sql-explorer/pull/722
 .. _#713: https://github.com/explorerhq/sql-explorer/pull/713
 .. _#710: https://github.com/explorerhq/sql-explorer/pull/710
 

explorer/tests/test_views.py

@@ -1,6 +1,7 @@
 import importlib
 import json
 import time
+from datetime import timedelta
 import unittest
 import os
 from unittest.mock import Mock, patch, MagicMock
@@ -10,11 +11,13 @@
 from django.core.files.uploadedfile import SimpleUploadedFile
 from django.core.cache import cache
 from django.core.exceptions import ImproperlyConfigured
-from django.db import DatabaseError, OperationalError
+from django.db import DatabaseError, OperationalError, connection
 from django.forms.models import model_to_dict
 from django.shortcuts import redirect
 from django.test import TestCase
+from django.test.utils import CaptureQueriesContext
 from django.urls import reverse
+from django.utils import timezone
 
 from explorer import app_settings
 from explorer.forms import QueryForm
@@ -77,6 +80,62 @@ def test_run_count(self):
         resp = self.client.get(reverse("explorer_index"))
         self.assertContains(resp, "4</td>")
 
+    def _log_at(self, run_at, **kwargs):
+        # run_at is auto_now_add, so set it after creation
+        ql = QueryLogFactory(**kwargs)
+        QueryLog.objects.filter(pk=ql.pk).update(run_at=run_at)
+        ql.refresh_from_db()
+        return ql
+
+    def _list_row(self, resp, query):
+        return next(o for o in resp.context["object_list"] if not o["is_header"] and o["id"] == query.id)
+
+    def test_last_run_status(self):
+        never_run = SimpleQueryFactory(title="never run")
+        failed = SimpleQueryFactory(title="failed last")
+        self._log_at(timezone.now() - timedelta(days=1), query=failed, success=True)
+        last = self._log_at(timezone.now(), query=failed, success=False)
+        resp = self.client.get(reverse("explorer_index"))
+
+        row = self._list_row(resp, never_run)
+        self.assertEqual(row["run_count"], 0)
+        self.assertTrue(row["ran_successfully"])
+        self.assertEqual(row["last_run_at"], never_run.created_at)
+
+        row = self._list_row(resp, failed)
+        self.assertEqual(row["run_count"], 2)
+        self.assertFalse(row["ran_successfully"])
+        self.assertEqual(row["last_run_at"], last.run_at)
+
+    def test_recently_viewed_is_latest_run_of_distinct_queries(self):
+        q1 = SimpleQueryFactory(title="first")
+        q2 = SimpleQueryFactory(title="second")
+        other_user = User.objects.create_user("other", "other@other.com", "pwd")
+        now = timezone.now()
+        self._log_at(now - timedelta(minutes=3), query=q1, run_by_user=self.user)
+        self._log_at(now - timedelta(minutes=2), query=q2, run_by_user=self.user)
+        latest_q1 = self._log_at(now - timedelta(minutes=1), query=q1, run_by_user=self.user)
+        self._log_at(now, query=q2, run_by_user=other_user)
+        resp = self.client.get(reverse("explorer_index"))
+        recent = resp.context["recent_queries"]
+        self.assertEqual([ql.query_id for ql in recent], [q1.id, q2.id])
+        self.assertEqual(recent[0].id, latest_q1.id)
+
+    def test_query_count_does_not_grow_with_queries_or_logs(self):
+        def count_sql_for_index():
+            with CaptureQueriesContext(connection) as ctx:
+                self.client.get(reverse("explorer_index"))
+            return len(ctx.captured_queries)
+
+        q = SimpleQueryFactory(title="one")
+        QueryLogFactory(query=q, run_by_user=self.user)
+        baseline = count_sql_for_index()
+        for i in range(5):
+            q = SimpleQueryFactory(title=f"more {i}")
+            for _ in range(3):
+                QueryLogFactory(query=q, run_by_user=self.user)
+        self.assertEqual(count_sql_for_index(), baseline)
+
 
 class TestQueryCreateView(TestCase):
 
@@ -852,6 +911,21 @@ def test_filter_logs_by_connection(self):
         self.assertContains(resp, "select other conn;")
         self.assertContains(resp, "select default conn;")
 
+    def test_log_page_query_count_does_not_grow_with_rows(self):
+        def count_sql_for_logs():
+            with CaptureQueriesContext(connection) as ctx:
+                self.client.get(reverse("explorer_logs"))
+            return len(ctx.captured_queries)
+
+        QueryLogFactory(run_by_user=self.user)
+        baseline = count_sql_for_logs()
+        for i in range(5):
+            conn = DatabaseConnection.objects.create(alias=f"logconn{i}", engine="django.db.backends.sqlite3",
+                                                     name=":memory:")
+            user = User.objects.create_user(f"logger{i}", f"logger{i}@example.com", "pwd")
+            QueryLogFactory(run_by_user=user, database_connection_id=conn.id)
+        self.assertEqual(count_sql_for_logs(), baseline)
+
     def test_filter_logs_run_by_me(self):
         other = User.objects.create_superuser("other", "other@other.com", "pwd")
         QueryLogFactory(sql="select mine;", run_by_user=self.user)

explorer/views/list.py

@@ -1,6 +1,7 @@
 import re
 from collections import Counter
 
+from django.db.models import Count, Max, OuterRef, Q, Subquery
 from django.forms.models import model_to_dict
 from django.views.generic import ListView
 
@@ -17,22 +18,25 @@ class ListQueryView(PermissionRequiredMixin, ExplorerContextMixin, ListView):
     model = Query
 
     def recently_viewed(self):
-        qll = QueryLog.objects.filter(
-            run_by_user=self.request.user, query_id__isnull=False
-        ).order_by(
-            "-run_at"
-        ).select_related("query")
-
-        ret = []
-        tracker = []
-        for ql in qll:
-            if len(ret) == app_settings.EXPLORER_RECENT_QUERY_COUNT:
-                break
-
-            if ql.query_id not in tracker:
-                ret.append(ql)
-                tracker.append(ql.query_id)
-        return ret
+        # The most recent run of each of the user's N most recently run queries. Done in the database: iterating the
+        # user's logs in Python loaded every one of them, which got slow as logs accumulated.
+        latest_runs = (
+            QueryLog.objects.filter(run_by_user=self.request.user, query_id__isnull=False)
+            .values("query_id")
+            .annotate(last_run_at=Max("run_at"))
+            .order_by("-last_run_at")[:app_settings.EXPLORER_RECENT_QUERY_COUNT]
+        )
+        latest_runs = list(latest_runs)
+        if not latest_runs:
+            return []
+        match = Q()
+        for run in latest_runs:
+            match |= Q(query_id=run["query_id"], run_at=run["last_run_at"])
+        logs = {
+            ql.query_id: ql
+            for ql in QueryLog.objects.filter(match, run_by_user=self.request.user).select_related("query")
+        }
+        return [logs[run["query_id"]] for run in latest_runs if run["query_id"] in logs]
 
     def get_context_data(self, **kwargs):
         context = super().get_context_data(**kwargs)
@@ -44,18 +48,16 @@ def get_context_data(self, **kwargs):
         return context
 
     def get_queryset(self):
-        if app_settings.EXPLORER_PERMISSION_VIEW(self.request):
-            qs = (
-                Query.objects.prefetch_related(
-                    "created_by_user", "querylog_set"
-                ).all()
-            )
-        else:
-            qs = (
-                Query.objects.prefetch_related(
-                    "created_by_user", "querylog_set"
-                ).filter(pk__in=allowed_query_pks(self.request.user.id))
-            )
+        # Run counts and the last run are computed in the database rather than by prefetching every QueryLog, which
+        # loaded the entire log history into memory on each page view.
+        last_log = QueryLog.objects.filter(query=OuterRef("pk")).order_by("-run_at")
+        qs = Query.objects.select_related("created_by_user", "database_connection").annotate(
+            run_count=Count("querylog"),
+            last_run_at=Subquery(last_log.values("run_at")[:1]),
+            last_run_success=Subquery(last_log.values("success")[:1]),
+        ).order_by("title")  # Meta.ordering isn't applied once the query has an aggregate
+        if not app_settings.EXPLORER_PERMISSION_VIEW(self.request):
+            qs = qs.filter(pk__in=allowed_query_pks(self.request.user.id))
         return qs
 
     def _build_queries_and_headers(self):
@@ -108,16 +110,16 @@ def _build_queries_and_headers(self):
                 })
                 rendered_headers.append(header)
 
-            lrl = q.last_run_log()
             model_dict.update({
                 "is_in_category": headers[header] > 1,
                 "collapse_target": collapse_target,
                 "created_at": q.created_at,
                 "is_header": False,
-                "run_count": q.querylog_set.count(),
+                "run_count": q.run_count,
                 "connection_name": str(q.database_connection),
-                "ran_successfully": lrl.success,
-                "last_run_at": lrl.run_at,
+                # Matches Query.last_run_log(): a never-run query shows its creation time as a successful "run"
+                "ran_successfully": q.last_run_success if q.last_run_at else True,
+                "last_run_at": q.last_run_at or q.created_at,
                 "created_by_user":
                     str(q.created_by_user) if q.created_by_user else None,
                 "is_favorite": q.id in query_favorites_for_user
@@ -141,7 +143,7 @@ def get_queryset(self):
             kwargs["database_connection_id"] = connection_id
         if self.request.GET.get("run_by_me") and self.request.user.is_authenticated:
             kwargs["run_by_user"] = self.request.user
-        return QueryLog.objects.filter(**kwargs).all()
+        return QueryLog.objects.filter(**kwargs).select_related("run_by_user", "database_connection")
 
     def get_context_data(self, **kwargs):
         context = super().get_context_data(**kwargs)