From 23f0ae96662a7d7d2083828a916ee0710afd6b3b Mon Sep 17 00:00:00 2001 From: Brian Pepple Date: Mon, 19 Oct 2026 08:19:07 -0400 Subject: [PATCH 1/4] Count series issues with a subquery in the API The series list, series detail, and publisher series_list endpoints annotated num_issues with Count("issues", distinct=True). That joins and groups every issue row before sorting and paginating, and the pagination COUNT(*) repeats the join. On production-sized data (184k issues) this took ~450ms for /api/series/ and ~200ms for a large publisher's series_list on a cache miss. Use a correlated subquery instead. Postgres evaluates it only for the rows on the requested page, and the pagination COUNT(*) leaves it out. On the same data /api/series/ dropped to ~5ms of SQL and series_list to ~4ms. Add a test covering issue_count for series with and without issues. --- api/views.py | 30 +++++++++++++++++++++++------- tests/comicsdb/test_api_series.py | 31 ++++++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/api/views.py b/api/views.py index a5a42a24..f6f0057c 100644 --- a/api/views.py +++ b/api/views.py @@ -8,11 +8,14 @@ DecimalField, F, IntegerField, + OuterRef, Prefetch, Q, + Subquery, Sum, When, ) +from django.db.models.functions import Coalesce from django.http import Http404 from django.shortcuts import get_object_or_404 from django.utils import timezone @@ -116,6 +119,20 @@ from users.models import CustomUser from wish_list.models import WishList, WishListItem +# Correlated per-series issue count. Unlike Count("issues") -- which joins and +# GROUPs BY every issue row before sorting and paginating -- Postgres only +# evaluates this for the rows actually returned, and the pagination COUNT(*) +# drops it entirely. Coalesce because a series with no issues yields no row. +_series_issue_count = Coalesce( + Subquery( + Issue.objects.filter(series=OuterRef("pk")) + .values("series") + .annotate(count=Count("pk")) + .values("count") + ), + 0, +) + class ReadingListItemsPagination(PageNumberPagination): """Custom pagination for reading list items with 50 items per page.""" @@ -763,7 +780,7 @@ def series_list(self, request, pk=None): queryset = ( publisher.series.select_related("series_type", "publisher") - .annotate(num_issues=Count("issues", distinct=True)) + .annotate(num_issues=_series_issue_count) .order_by("sort_name", "year_began") ) page = self.paginate_queryset(queryset) @@ -819,24 +836,23 @@ class SeriesViewSet( cache_detail_dependent_labels = (ModelLabel.PUBLISHER_NAME, ModelLabel.IMPRINT_NAME) def get_modified_queryset(self): - # get_queryset() annotates num_issues for list/retrieve -- an - # aggregate that survives into (pk, modified) values_list() as an - # extra JOIN + GROUP BY even though the annotated field isn't - # selected. This lookup only needs the row's own pk/modified. + # get_queryset() adds select_related joins and a num_issues + # subquery for list/retrieve. This lookup only needs the row's own + # pk/modified. return Series.objects.all() def get_queryset(self): queryset = super().get_queryset() match self.action: case "list": - queryset = queryset.annotate(num_issues=Count("issues", distinct=True)).order_by( + queryset = queryset.annotate(num_issues=_series_issue_count).order_by( "sort_name", "year_began" ) case "retrieve": queryset = ( queryset.select_related("imprint") .prefetch_related("genres", "associated") - .annotate(num_issues=Count("issues", distinct=True)) + .annotate(num_issues=_series_issue_count) ) return queryset diff --git a/tests/comicsdb/test_api_series.py b/tests/comicsdb/test_api_series.py index e1e893ee..a4079ca7 100644 --- a/tests/comicsdb/test_api_series.py +++ b/tests/comicsdb/test_api_series.py @@ -5,10 +5,11 @@ import pytest from django.db.models import Count, Q from django.urls import reverse +from django.utils import timezone from rest_framework import status from api.v1_0.serializers import SeriesListSerializer -from comicsdb.models import Credits, Series +from comicsdb.models import Credits, Issue, Series from comicsdb.models.character import Character from comicsdb.models.creator import Creator from comicsdb.models.credits import Role @@ -393,3 +394,31 @@ def test_list_series_year_end_set( assert resp.status_code == status.HTTP_200_OK completed = next(r for r in resp.data["results"] if r["year_end"] is not None) assert completed["year_end"] == 2005 + + +def test_issue_count( + api_client_with_credentials, create_user, dc_comics, fc_series: Series, bat_sups_series +): + user = create_user() + for number in ("1", "2"): + Issue.objects.create( + series=fc_series, + number=number, + cover_date=timezone.now().date(), + edited_by=user, + created_by=user, + ) + expected = {fc_series.id: 2, bat_sups_series.id: 0} + + for url in ( + reverse("api:series-list"), + reverse("api:publisher-series-list", kwargs={"pk": dc_comics.pk}), + ): + resp = api_client_with_credentials.get(url) + assert resp.status_code == status.HTTP_200_OK + assert {r["id"]: r["issue_count"] for r in resp.data["results"]} == expected + + for pk, count in expected.items(): + resp = api_client_with_credentials.get(reverse("api:series-detail", kwargs={"pk": pk})) + assert resp.status_code == status.HTTP_200_OK + assert resp.data["issue_count"] == count From 40e135ff9518373209791c380600bdfb9e9aab53 Mon Sep 17 00:00:00 2001 From: Brian Pepple Date: Mon, 19 Oct 2026 08:23:35 -0400 Subject: [PATCH 2/4] Make series API list ordering deterministic /api/series/ and publisher series_list ordered by (sort_name, year_began), which isn't unique: 506 series share a value with at least one other series. Postgres may return tied rows in a different order on each query, so a client paginating the list could see a series twice and miss another. Add id as a final tiebreaker. It has no measurable cost, since the existing (sort_name, year_began) index still drives the sort. --- api/views.py | 10 ++++++++-- tests/comicsdb/test_api_series.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/api/views.py b/api/views.py index f6f0057c..456fe4fc 100644 --- a/api/views.py +++ b/api/views.py @@ -133,6 +133,12 @@ 0, ) +# (sort_name, year_began) isn't unique -- e.g. two "Miles Morales: Spider-Man" +# series began in 2019 -- and Postgres may return tied rows in a different +# order per query, so paginated clients could see one twice and miss the +# other. "id" makes the order total. +_SERIES_LIST_ORDERING = ("sort_name", "year_began", "id") + class ReadingListItemsPagination(PageNumberPagination): """Custom pagination for reading list items with 50 items per page.""" @@ -781,7 +787,7 @@ def series_list(self, request, pk=None): queryset = ( publisher.series.select_related("series_type", "publisher") .annotate(num_issues=_series_issue_count) - .order_by("sort_name", "year_began") + .order_by(*_SERIES_LIST_ORDERING) ) page = self.paginate_queryset(queryset) if page is None: @@ -846,7 +852,7 @@ def get_queryset(self): match self.action: case "list": queryset = queryset.annotate(num_issues=_series_issue_count).order_by( - "sort_name", "year_began" + *_SERIES_LIST_ORDERING ) case "retrieve": queryset = ( diff --git a/tests/comicsdb/test_api_series.py b/tests/comicsdb/test_api_series.py index a4079ca7..91d8de38 100644 --- a/tests/comicsdb/test_api_series.py +++ b/tests/comicsdb/test_api_series.py @@ -422,3 +422,32 @@ def test_issue_count( resp = api_client_with_credentials.get(reverse("api:series-detail", kwargs={"pk": pk})) assert resp.status_code == status.HTTP_200_OK assert resp.data["issue_count"] == count + + +def test_list_orders_ties_by_id( + api_client_with_credentials, create_user, dc_comics, single_issue_type +): + user = create_user() + tied = [ + Series.objects.create( + name="Miles Morales: Spider-Man", + sort_name="Miles Morales: Spider-Man", + publisher=dc_comics, + volume=volume, + year_began=2019, + series_type=single_issue_type, + status=Series.Status.ONGOING, + edited_by=user, + created_by=user, + ) + for volume in (1, 2) + ] + expected = sorted(s.id for s in tied) + + for url in ( + reverse("api:series-list"), + reverse("api:publisher-series-list", kwargs={"pk": dc_comics.pk}), + ): + resp = api_client_with_credentials.get(url) + assert resp.status_code == status.HTTP_200_OK + assert [r["id"] for r in resp.data["results"]] == expected From 5d5e8b8e2397df2b11f8cfdba87fc592447af060 Mon Sep 17 00:00:00 2001 From: Brian Pepple Date: Mon, 19 Oct 2026 08:27:35 -0400 Subject: [PATCH 3/4] Break ordering ties by id on core metadata models Arc, Character, Creator, Imprint, Publisher, Team, Universe, Series, and Issue all ordered by fields that aren't unique (e.g. 129 characters share a name with another character). Postgres may return tied rows in a different order on each query, so paginated API and web lists could show a row twice and miss another. Add id as a final ordering key on each model. This also covers the issue_list actions, whose order_by("series") expands to Series' Meta.ordering. The migration only changes model options and runs no SQL. --- .../0061_ordering_tiebreak_by_id.py | 51 +++++++++++++++++++ comicsdb/models/arc.py | 2 +- comicsdb/models/character.py | 2 +- comicsdb/models/creator.py | 2 +- comicsdb/models/imprint.py | 2 +- comicsdb/models/issue.py | 2 +- comicsdb/models/publisher.py | 2 +- comicsdb/models/series.py | 2 +- comicsdb/models/team.py | 2 +- comicsdb/models/universe.py | 2 +- 10 files changed, 60 insertions(+), 9 deletions(-) create mode 100644 comicsdb/migrations/0061_ordering_tiebreak_by_id.py diff --git a/comicsdb/migrations/0061_ordering_tiebreak_by_id.py b/comicsdb/migrations/0061_ordering_tiebreak_by_id.py new file mode 100644 index 00000000..969eac36 --- /dev/null +++ b/comicsdb/migrations/0061_ordering_tiebreak_by_id.py @@ -0,0 +1,51 @@ +# Generated by Django 6.1.2 on 2026-10-19 12:24 + +from django.db import migrations + + +class Migration(migrations.Migration): + dependencies = [ + ("comicsdb", "0060_attribution_ordering"), + ] + + operations = [ + migrations.AlterModelOptions( + name="arc", + options={"ordering": ["name", "id"]}, + ), + migrations.AlterModelOptions( + name="character", + options={"ordering": ["name", "id"]}, + ), + migrations.AlterModelOptions( + name="creator", + options={"ordering": ["name", "id"]}, + ), + migrations.AlterModelOptions( + name="imprint", + options={"ordering": ["name", "id"]}, + ), + migrations.AlterModelOptions( + name="issue", + options={"ordering": ["series__sort_name", "cover_date", "store_date", "number", "id"]}, + ), + migrations.AlterModelOptions( + name="publisher", + options={"ordering": ["name", "id"]}, + ), + migrations.AlterModelOptions( + name="series", + options={ + "ordering": ["sort_name", "year_began", "id"], + "verbose_name_plural": "Series", + }, + ), + migrations.AlterModelOptions( + name="team", + options={"ordering": ["name", "id"]}, + ), + migrations.AlterModelOptions( + name="universe", + options={"ordering": ["name", "designation", "id"]}, + ), + ] diff --git a/comicsdb/models/arc.py b/comicsdb/models/arc.py index c17c8641..fb00f63c 100644 --- a/comicsdb/models/arc.py +++ b/comicsdb/models/arc.py @@ -57,7 +57,7 @@ class Meta: models.Index(fields=["cv_id"], name="arc_cv_id_idx"), models.Index(fields=["gcd_id"], name="arc_gcd_id_idx"), ] - ordering = ["name"] + ordering = ["name", "id"] pre_save.connect(pre_save_slug, sender=Arc, dispatch_uid="pre_save_arc") diff --git a/comicsdb/models/character.py b/comicsdb/models/character.py index 1e1983f9..b9292311 100644 --- a/comicsdb/models/character.py +++ b/comicsdb/models/character.py @@ -81,7 +81,7 @@ class Meta: models.Index(fields=["cv_id"], name="character_cv_id_idx"), models.Index(fields=["gcd_id"], name="character_gcd_id_idx"), ] - ordering = ["name"] + ordering = ["name", "id"] pre_save.connect(pre_save_slug, sender=Character, dispatch_uid="pre_save_character") diff --git a/comicsdb/models/creator.py b/comicsdb/models/creator.py index 81b47bb4..3b28379f 100644 --- a/comicsdb/models/creator.py +++ b/comicsdb/models/creator.py @@ -73,7 +73,7 @@ class Meta: models.Index(fields=["cv_id"], name="creator_cv_id_idx"), models.Index(fields=["gcd_id"], name="creator_gcd_id_idx"), ] - ordering = ["name"] + ordering = ["name", "id"] pre_save.connect(pre_save_slug, sender=Creator, dispatch_uid="pre_save_creator") diff --git a/comicsdb/models/imprint.py b/comicsdb/models/imprint.py index 59830a17..9d1c4eba 100644 --- a/comicsdb/models/imprint.py +++ b/comicsdb/models/imprint.py @@ -68,7 +68,7 @@ class Meta: models.Index(fields=["cv_id"], name="imprint_cv_id_idx"), models.Index(fields=["gcd_id"], name="imprint_gcd_id_idx"), ] - ordering = ["name"] + ordering = ["name", "id"] pre_save.connect(pre_save_slug, sender=Imprint, dispatch_uid="pre_save_imprint") diff --git a/comicsdb/models/issue.py b/comicsdb/models/issue.py index 0c544bea..d567bdcb 100644 --- a/comicsdb/models/issue.py +++ b/comicsdb/models/issue.py @@ -187,7 +187,7 @@ class Meta: models.Index(fields=["cv_id"], name="issue_cv_id_idx"), models.Index(fields=["gcd_id"], name="issue_gcd_id_idx"), ] - ordering = ["series__sort_name", "cover_date", "store_date", "number"] + ordering = ["series__sort_name", "cover_date", "store_date", "number", "id"] unique_together = ["series", "number"] diff --git a/comicsdb/models/publisher.py b/comicsdb/models/publisher.py index ed6b39f7..3b99b343 100644 --- a/comicsdb/models/publisher.py +++ b/comicsdb/models/publisher.py @@ -81,7 +81,7 @@ class Meta: models.Index(fields=["cv_id"], name="publisher_cv_id_idx"), models.Index(fields=["gcd_id"], name="publisher_gcd_id_idx"), ] - ordering = ["name"] + ordering = ["name", "id"] pre_save.connect(pre_save_slug, sender=Publisher, dispatch_uid="pre_save_publisher") diff --git a/comicsdb/models/series.py b/comicsdb/models/series.py index 30d49767..443f5c87 100644 --- a/comicsdb/models/series.py +++ b/comicsdb/models/series.py @@ -123,7 +123,7 @@ class Meta: models.Index(fields=["cv_id"], name="series_cv_id_idx"), models.Index(fields=["gcd_id"], name="series_gcd_id_idx"), ] - ordering = ["sort_name", "year_began"] + ordering = ["sort_name", "year_began", "id"] unique_together = ["publisher", "name", "volume", "series_type"] verbose_name_plural = _("Series") diff --git a/comicsdb/models/team.py b/comicsdb/models/team.py index b188ebf4..db6af443 100644 --- a/comicsdb/models/team.py +++ b/comicsdb/models/team.py @@ -69,7 +69,7 @@ class Meta: models.Index(fields=["cv_id"], name="team_cv_id_idx"), models.Index(fields=["gcd_id"], name="team_gcd_id_idx"), ] - ordering = ["name"] + ordering = ["name", "id"] pre_save.connect(pre_save_slug, sender=Team, dispatch_uid="pre_save_team") diff --git a/comicsdb/models/universe.py b/comicsdb/models/universe.py index 59d860ea..56e47eeb 100644 --- a/comicsdb/models/universe.py +++ b/comicsdb/models/universe.py @@ -78,7 +78,7 @@ class Meta: models.Index(fields=["cv_id"], name="universe_cv_id_idx"), models.Index(fields=["gcd_id"], name="universe_gcd_id_idx"), ] - ordering = ["name", "designation"] + ordering = ["name", "designation", "id"] unique_together = ["publisher", "name", "designation"] db_table_comment = "Publisher Universes" From d648d5b9321fa49d22e0973c4df94e806a07cbc5 Mon Sep 17 00:00:00 2001 From: Brian Pepple Date: Mon, 19 Oct 2026 08:28:43 -0400 Subject: [PATCH 4/4] Drop the now-redundant explicit series ordering from the API views. --- api/views.py | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/api/views.py b/api/views.py index 456fe4fc..80cb616b 100644 --- a/api/views.py +++ b/api/views.py @@ -133,12 +133,6 @@ 0, ) -# (sort_name, year_began) isn't unique -- e.g. two "Miles Morales: Spider-Man" -# series began in 2019 -- and Postgres may return tied rows in a different -# order per query, so paginated clients could see one twice and miss the -# other. "id" makes the order total. -_SERIES_LIST_ORDERING = ("sort_name", "year_began", "id") - class ReadingListItemsPagination(PageNumberPagination): """Custom pagination for reading list items with 50 items per page.""" @@ -784,10 +778,8 @@ def series_list(self, request, pk=None): if cached is not None: return _mark_cache_status(Response(cached), hit=True) - queryset = ( - publisher.series.select_related("series_type", "publisher") - .annotate(num_issues=_series_issue_count) - .order_by(*_SERIES_LIST_ORDERING) + queryset = publisher.series.select_related("series_type", "publisher").annotate( + num_issues=_series_issue_count ) page = self.paginate_queryset(queryset) if page is None: @@ -851,9 +843,7 @@ def get_queryset(self): queryset = super().get_queryset() match self.action: case "list": - queryset = queryset.annotate(num_issues=_series_issue_count).order_by( - *_SERIES_LIST_ORDERING - ) + queryset = queryset.annotate(num_issues=_series_issue_count) case "retrieve": queryset = ( queryset.select_related("imprint")