fix(loader): seek taxonomy-filtered listings via a denormalized pivot#1962
Conversation
Denormalize the filter+sort columns onto content_taxonomies, mirror the ec_* sort indexes onto the pivot, and drive taxonomy-filtered listings from the pivot with an authoritative ec_* re-check. Fixes full-collection scans (~75k D1 rows read for a one-row page) on selective terms (emdash-cms#1834). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 3adf276 The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
Bot
added
review/needs-review
Scope checkThis PR changes 1,228 lines across 13 files. Large PRs are harder to review and more likely to be closed without review. If this scope is intentional, no action needed. A maintainer will review it. If not, please consider splitting this into smaller PRs. See CONTRIBUTING.md for contribution guidelines. |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
github-actions
Bot
added
the
query-count changed
Query-count snapshot changes8 routes changed, total Δ +8 queries. SQLite
D1
Comparing snapshot files between base and head. Updated automatically on each push. |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the right change for #1834. Taxonomy-filtered listings were indeed doing a full walk of ec_* with a per-row correlated EXISTS; denormalizing the filter/sort columns onto content_taxonomies and driving the query from a covering pivot index is an idiomatic fix for D1's stats-blind planner. The read path keeps ec_* authoritative by re-checking deleted_at/status/locale on the joined row, which matches the documented non-atomic-write staleness semantics.
I traced:
- Schema & migration (051): six nullable denormalized columns added idempotently, backfilled per-collection with a safe correlated
UPDATE, and four composite indexes built after the backfill. Thedownpath is present for tests. - Write path:
ContentRepository.restampEntryPivotis awaited after everyec_*mutation that moves a denormalized column (status, published/scheduled dates, deleted_at).TaxonomyRepositorystamps the pivot at every insert site (attachToEntry,setTermsForEntry,copyEntryTerms). Production insert paths all go through these helpers. - Read path:
buildTaxonomyPivotQueryproduces an indexed sort forpublished_at/created_at, a bounded temp-sort for other orders, handles OR-within-taxonomy (GROUP BY/MAX/cursor inHAVING), multi-taxonomy AND via residualEXISTS, and carries byline filters in the CTE. Inputs are parameterized or validated (getTableName/validateIdentifier), and no user strings reachsql.raw. - Tests: new unit tests cover parity, locale/status filtering, stale-pivot guards, re-stamp sync, keyset pagination, byline AND, and admin-shape builders; a new integration plan test asserts the expected SQLite index usage.
No blocking bugs or AGENTS.md violations. One note, not a code finding: the PR description references docs/superpowers/specs/2026-07-11-taxonomy-filter-pivot-seek-design.md, but that file is not in the tree.
LGTM.
Overlapping PRsThis PR modifies files that are also changed by other open PRs:
This may cause merge conflicts or duplicated work. A maintainer will coordinate. |
github-actions
Bot
removed
the
review/needs-review
There was a problem hiding this comment.
Re-review of the same pivot-drive taxonomy listing change. The approach remains sound and idiomatic for #1834: denormalizing filter/sort columns onto content_taxonomies, mirroring the composite sort indexes, and driving the listing from a covering pivot index lets D1/SQLite short-circuit on LIMIT while keeping ec_* authoritative via the outer re-check.
I re-read the new migration, loader builder, write-path re-stamps, and tests. No new blocking bugs or AGENTS.md violations surfaced:
- Migration 051 is idempotent on column adds and index creation, uses validated identifiers, and the row-value correlated
UPDATEis valid on both SQLite and Postgres. ContentRepository.updatecorrectly avoids re-stamping on plain data edits (noupdated_atdenormalization), and re-stamps whereverstatus,published_at, orscheduled_atcan change, plus soft-delete/restore/publish/schedule/unschedule.TaxonomyRepositorystamps the pivot at every insert/assignment site.- The read path re-checks
deleted_at,status, andlocaleon the joinedec_*row, matching the documented non-atomic-write semantics. - Tests cover parity, stale-pivot guards, locale/status filtering, keyset pagination, and query-plan shape.
The only note from my prior review still stands, and it is non-blocking: the PR description references docs/superpowers/specs/2026-07-11-taxonomy-filter-pivot-seek-design.md, but that file is not present in the tree. If the spec is meant to land with the change, it should be added; otherwise the PR description should be updated to remove the reference.
LGTM.
…emdash-cms#1962) * fix(loader): seek taxonomy-filtered listings via a denormalized pivot Denormalize the filter+sort columns onto content_taxonomies, mirror the ec_* sort indexes onto the pivot, and drive taxonomy-filtered listings from the pivot with an authoritative ec_* re-check. Fixes full-collection scans (~75k D1 rows read for a one-row page) on selective terms (emdash-cms#1834). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * ci: update query-count snapshots --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: emdashbot[bot] <emdashbot[bot]@users.noreply.github.com>
2 participants
What does this PR do?
Taxonomy-filtered collection listings (
getEmDashCollection("posts", { where: { category }, orderBy, limit })) used to apply the taxonomy filter as a correlatedEXISTSonSELECT * FROM ec_<collection> … ORDER BY … LIMIT ?. The pivot tablecontent_taxonomiescarried only(collection, entry_id, taxonomy_id), so the filter (deleted_at/status/locale) and the ordering (published_at/created_at) could only be evaluated on theec_*row. On stats-blind SQLite/D1 a selective term never fills theLIMIT, so the query walks the entire collection — a measured ~75,000 D1 rows read for a page returning one row (rows-read is the billed/throttled unit on D1).This denormalizes the filter + sort columns onto
content_taxonomies, mirrorsec_*'s composite sort indexes onto the pivot keyed by(taxonomy_id, collection, deleted_at, [locale,] <sort> DESC, entry_id DESC), and restructures the taxonomy-filtered listing to drive from the pivot: seek the term, walk a sort-ordered pivot index, letLIMITshort-circuit, and touchec_*only by primary key to hydrate the page.The pivot is advisory;
ec_*is authoritative. D1 has no transactions, so the write-path re-stamp is a separate statement from theec_*mutation and can be transiently stale. The outerec_*join therefore re-checksdeleted_at/status/localeon the joined row — a stale pivot can only cause a page to briefly under-fill (self-healing on the next request); it can never leak a deleted or wrong-status/locale row onto a public listing.Implementation:
content_taxonomies(status,scheduled_at,deleted_at,locale,published_at,created_at).updated_atis deliberately not denormalized — it moves on every edit, so denormalizing it would force a pivot re-stamp on the common edit path;updated_at-sorted taxonomy listings take a bounded temp-sort path instead.loader.ts): a caller-agnostic pivot-drive builder (parameterized on thedeleted_atpredicate and status condition, so it can also serve an admin trash/all-statuses shape). Both dialects run the identical query shape.ContentRepository,TaxonomyRepository) — no stray SQL.Supersedes the closed #1852 (cached-count runtime planner — wrong layer). This is also the substrate the future taxonomy-subtree filter needs to perform well.
Follow-up (out of scope):
idx_content_taxonomies_term (taxonomy_id)from migration 048 is now redundant — all four new indexes lead withtaxonomy_idand serve a baretaxonomy_idseek as a prefix. Dropping it is a separate additive-discipline call (048 was a deliberate restore for #1701), flagged here so the write cost isn't paid twice.Closes #1834
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runAI-generated code disclosure
Screenshots / test output
emdashcore suite: 4675 passed, 3 skipped. Lint: 0 diagnostics. Typecheck clean. Formatted.tests/unit/loader-taxonomy-pivot.test.ts(describeEachDialect: parity across selective/broad/OR-within-taxonomy/multi-term-AND/sort orders/locale/status; stale-pivot correctness under non-atomic writes; re-stamp sync; keyset pagination incl. the GROUP-BY/HAVING multi-group path; byline-in-pivot; admin trash/all-statuses builder shapes) andtests/integration/loader-taxonomy-pivot-plan.test.ts(SQLiteEXPLAIN QUERY PLAN: asserts the correct pivot index per case, noec_*full scan, and no temp B-tree for the indexed early-LIMIT).EMDASH_TEST_PGwas not set locally, so only the SQLite dialect executed. The raw SQL (row-valueUPDATE … SET (cols) = (SELECT …),MAX()/GROUP BY/HAVING, CTE) was reasoned through as valid Postgres but not exercised — please confirm thedescribeEachDialectsuite runs against real Postgres in CI.+1query on the taxonomy-filtered routes (/category/*,/tag/*), on both cold and warm. This is the pivot-drive's up-front term resolution (resolveTermGroups): the seek needs literaltranslation_groupvalues, because on a stats-blind planner an inlinedtaxonomy_id IN (SELECT …)is treated as multi-valued and forces a temp-sort over the whole term — defeating the early-LIMITseek (confirmed viaEXPLAIN QUERY PLAN). So the values must be resolved in a separate statement. It's cheap in rows-read (ridesidx_taxonomies_name_locale, touches one taxonomy's terms), but currently uncached, so it lands on cold and warm. Caching that resolution to reclaim the warm cost is deferred — the obvious reuse (getTaxonomyTerms) regresses localized filtering (Collectionwheretaxonomy filter joins ont.id(term id) instead oftranslation_group, so it only matches default-locale term slugs #1480), so the right shape is an open call tracked in Taxonomy-filtered listings: how should we cache the term→group resolution? (follow-up to #1962) #1966. Net rows-read still collapses by ~4 orders of magnitude on a selective term page, which is the point of this PR.🤖 Generated with Claude Code