Photo list query has no index for its sort shape (436x) #84
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Severity: HIGH (measured)
The problem
PhotoRepository.list_photosfilters oncollection_idand orders bycreated_at, but no indexcontains
created_atfor the photo table. Every page pays a full scan plus a temporary B-treesort.
Measurements
Built a 50k-row SQLite database mirroring migration
001exactly, then ranEXPLAIN QUERY PLANand timings:
SCAN photo+ temp B-treeSCAN photo+ temp B-tree(collection_id, created_at)(collection_id, status, created_at)436x on the hottest query. The cost is linear in collection size, and keyset pagination does
not amortize it without the index.
Fix
One Alembic migration:
ix_photo_collection_created (collection_id, created_at)ix_photo_collection_statuswith(collection_id, status, created_at)Both plans were verified to flip to index-order
SEARCHwith no temp B-tree.Done when
EXPLAIN QUERY PLANshows no temp B-tree for either shapeReferences
backend/app/repositories/photos.py:40-54backend/app/models/models.py:262-267backend/alembic/versions/001_initial_schema.py:137-140Related: #48 asks generically to verify indexes; this is the concrete answer.
Done in
c225acb. Migration010, plusbackend/tests/test_query_plans.py.Re-measured, because the issue's numbers predate #88
#88 changed the ordering to
(created_at, id)with a row-value cursor comparison, so I rebuilt the 50k-row database and measured the current query rather than trusting the earlier figures. Best of five:SCAN+ temp B-tree → index orderSCAN+ temp B-tree → index orderstatus=pending(55%)status=disputed(2%)(The absolute numbers are lower than the issue's — different machine — but the plan change is identical, and the deep page is the interesting one: it was the slowest of the four, because keyset pagination does not amortize a missing index. The cursor predicate still had to be applied to every row.)
Two corrections to the proposed shape, both from measuring
1.
idhas to be in the index. With(collection_id, created_at)the plan still reportsUSE TEMP B-TREE FOR LAST TERM OF ORDER BY— the sort is smaller, not gone, because #88 made the sort key(created_at, id). So the index is(collection_id, created_at, id).2. The status index needs the sort key too. Given only the created index, SQLite drops
ix_photo_collection_statusentirely and filters:status=pending(55% of rows) → 0.140 ms, finestatus=disputed(2% of rows) → 0.375 ms, about 2.5× the indexed answerdisputedis the queue a reviewer opens most often, so the status index is widened to(collection_id, status, created_at, id)rather than left to be ignored. The narrow(collection_id, status)is dropped rather than kept alongside — it is a prefix of the new one, so it would cost writes and disk to answer nothing.A test that asserts the plan, not the timing
An index is not a behaviour. Dropped, every other test in the suite would keep passing — a little more slowly — until a collection grew large enough for someone to complain.
test_query_plans.pyassertsEXPLAIN QUERY PLANhas noTEMP B-TREEand names the expected index, that the deep page seeks on(created_at,id)>rather than filtering, and that the indexes exist on the migrated schema.That last one matters for the same reason #67's trigger test does: on SQLite
batch_alter_tableis create-copy-drop-rename, which takes a table's indexes with it — and an index would go that way with even less noise than a trigger.Verified load-bearing: against the pre-
010schema both plans come backUSE TEMP B-TREE FOR ORDER BY, so the assertions fail.Done when
EXPLAIN QUERY PLANshows no temp B-tree for either shape1028 passed, 8 skipped; migrations up and down from empty verified; ruff clean.