Skip to content

perf(roms): sort metadata fields on the indexed roms column - #4078

Merged
gantoine merged 2 commits into
rommapp:masterfrom
Spinnich:fix/metadata-sort-perf
Aug 4, 2026
Merged

perf(roms): sort metadata fields on the indexed roms column#4078
gantoine merged 2 commits into
rommapp:masterfrom
Spinnich:fix/metadata-sort-perf

Conversation

@Spinnich

@Spinnich Spinnich commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Description

Fixes #4067

Sorting a gallery by first_release_date took 20.3s against 0.27s for the
default sort on an 83k-game library. The issue guessed the cause was a missing
index, but the indexes were already there (idx_roms_generated_first_release_date
and friends, created by migration 0098). The real cause is where the sort key
sits
.

roms_metadata is not a table. It is a thin view over the STORED generated
columns on roms. Resolving one of its columns as the sort key did:

order_attr = getattr(RomMetadata, order_by)
query = query.outerjoin(RomMetadata, RomMetadata.rom_id == Rom.id)

That joins roms back to itself and leaves the sort key on the joined table.
MariaDB/MySQL can only use an index for ORDER BY when the sort column belongs
to the first table in the join order, and the right side of a LEFT JOIN
never can be. So the planner fell back to a full scan of roms plus a filesort
of the entire library, on every page, to return 72 rows.

The three view columns that are actually offered as sorts now resolve straight
to the indexed roms column and the join disappears. Measured on a 120k-row
copy of the real roms schema (169 MB), with Rom.metadatum's lazy="joined"
eager load present in both shapes so the comparison is honest:

Sort Before After
first_release_date ASC, LIMIT 72 6.02s 0.004s
average_rating DESC, LIMIT 72 2.16s 0.002s

EXPLAIN before: type: ALL, 113,534 rows, Using temporary; Using filesort.
After: type: index, key idx_roms_generated_first_release_date, 72 rows, no
filesort — the same plan shape as the default name_sort_key sort.

No migration, no new index, no schema change: the columns and their indexes
already existed, they just weren't being read.

Scope note: the remaining roms_metadata columns (genres, franchises,
collections, companies, game_modes, age_ratings) are JSON arrays with no
index to sort on, and nothing in the UI offers them as a sort. Since /api/roms
has no order_by allowlist and accepts any string, the view-join fallback is
deliberately kept so those keep behaving exactly as they do today.

Files changed

File What
backend/handler/database/roms_handler.py New ROM_METADATA_ORDER_COLUMNS map, plus a branch in get_roms_query() that resolves those three keys to the indexed roms column instead of joining the view.
backend/models/rom.py Maps generated_first_release_date, generated_average_rating and generated_player_count on Rom as read-only (FetchedValue()), so the sort has an attribute to reference.
backend/tests/handler/database/test_roms_metadata_sort.py New. 11 cases covering both the emitted SQL and the resulting order.

Testing notes

  • Full backend suite: 2715 passed, 2 skipped.
  • trunk fmt && trunk check clean.
  • End-to-end against a running app: all six sort paths return 200 with correct
    ordering (first_release_date, average_rating, player_count, each
    direction).
  • Perf numbers above measured on a 120k-row seeded roms table, since a normal
    dev library is too small to show the difference.

What a reviewer should look at

  1. The read-only mapping. generated_* are STORED generated columns owned by
    the engine, mapped with server_default/server_onupdate=FetchedValue() so
    SQLAlchemy never tries to write them. Worth confirming that reasoning holds:
    add_rom goes through session.merge(), update_rom uses a Core update()
    with caller-supplied keys, and _nullable_columns() is only ever called with
    RomFile/TrackMeta, so nothing enumerates Rom's columns to build a write.
  2. No API surface change. The response schemas in endpoints/responses/rom.py
    declare their fields explicitly, so the new attributes don't leak into
    OpenAPI and the frontend types are unchanged.
  3. Alembic. env.py already excludes any generated_* column from
    autogenerate, so adding them to the model shouldn't produce a spurious
    revision. I could not run alembic revision --autogenerate to prove this —
    it currently fails on master for an unrelated reason
    (NoReferencedTableError on saves.origin_device_id).
  4. player_count ordering is lexicographic (VARCHAR(100), so "10" < "2").
    That is unchanged: the view projects the same VARCHAR column, so this PR
    preserves the existing behavior rather than introducing it.

Out of scope, but found while in here: the RomUser branch of the same
function applies query.filter(RomUser.user_id == user_id) on top of an outer
join whose ON clause already carries that condition, which collapses it to an
effective inner join. Sorting by last_played therefore silently drops every ROM
with no rom_user row — 12,000 rows returned instead of 120,000 in my test data.
It is fast only because it discards 90% of the library. Left alone to keep this
PR focused; happy to open a separate issue.

Checklist

  • I've tested the changes locally
  • I've updated relevant comments
  • I've assigned reviewers for this PR
  • I've added unit tests that cover the changes

AI assistance disclosure

Per CONTRIBUTING.md: this change was written with AI assistance (Claude Code).
The AI performed the root-cause investigation, wrote the tests first, implemented
the fix, and ran the benchmarks and the full test suite. I reviewed the diff, the
benchmark methodology and the test coverage before opening this PR.

Ordering the gallery by a `roms_metadata` field joined the view back in and
sorted through it. `roms_metadata` is a thin view over STORED generated
columns on `roms` (migration 0098), so that join is `roms` to itself and it
leaves the sort key on a joined table, which no index can serve: the engine
filesorts the whole library for every page.

The generated columns are already indexed, so the three that are offered as
sorts now resolve straight to them and the join goes away. On a 120k-row
table this turns a full scan plus filesort into a 72-row index walk.

The remaining view columns are JSON arrays with no index and no sort that
offers them, so they keep reading the view.

Fixes rommapp#4067

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR optimizes metadata sorting by resolving three public sort keys directly to indexed generated columns on roms, avoiding an unnecessary view join.

  • Maps the existing generated release-date, rating, and player-count columns as server-managed ORM attributes.
  • Preserves the view-based fallback for other metadata fields.
  • Adds query-shape and result-ordering tests for the optimized paths.

Confidence Score: 5/5

The PR appears safe to merge with no actionable correctness or security issues identified.

The new sort expressions reference the same generated values and database types previously projected by the metadata view, while normal startup and test paths ensure the underlying migration is applied.

Important Files Changed

Filename Overview
backend/handler/database/roms_handler.py Redirects three metadata sort keys to equivalent indexed roms.generated_* columns while preserving existing fallback behavior.
backend/models/rom.py Maps existing STORED generated columns with matching types and server-managed value markers.
backend/tests/handler/database/test_roms_metadata_sort.py Covers optimized SQL shape, ascending and descending results, user joins, unmatched ROMs, and equivalence with view values.

Reviews (1): Last reviewed commit: "perf(roms): sort metadata fields on the ..." | Re-trigger Greptile

@Spinnich
Spinnich requested a review from gantoine August 3, 2026 02:27
Comment thread backend/handler/database/roms_handler.py Outdated
Comment thread backend/handler/database/roms_handler.py Outdated
Comment thread backend/models/rom.py Outdated
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Spinnich
Spinnich requested a review from gantoine August 4, 2026 12:15
@gantoine
gantoine merged commit d67dde2 into rommapp:master Aug 4, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Sorting a gallery by release date (or other metadata fields) is drastically slower than other sorts

2 participants