Replies: 1 comment
|
This is a good solution and we shoudl do it. Just a couple of things though: D1 doesn't support transactions, so the stamping is non-atomic. When querying we need to make sure that the queries also check the filter values on the ec_ rows as well as the pivot (i.e. it's in the outer query too). Inc;ude a test that e.g. sets publish on the pivot but soft-deletes the ec_ row, and makes sure the queries still filter it out. Also don't include the updated_at index. As you say, it's the biggest cost but the smallest value. Not worth it. |
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
This is a proposed solution for #1834
Problem
A taxonomy-filtered collection listing (
getEmDashCollection("posts", { where: { category }, orderBy, limit })) applies the taxonomy filter as a correlatedEXISTSonSELECT * FROM ec_<collection> … ORDER BY … LIMIT ?(packages/core/src/loader.ts:938-954,988-999). The pivot tablecontent_taxonomiescarries only(collection, entry_id, taxonomy_id), so the filter (deleted_at IS NULL,status,locale) and the ordering (published_at/created_at/updated_at) can only be evaluated on theec_*row. The planner therefore drives the scan from the collection's order index and probes the taxonomyEXISTSper row.On a stats-blind engine (SQLite/D1 never maintains
sqlite_stat1), for a selective term theLIMITnever fills, so the query walks the entire collection. Measured on a real dataset: ~75,000 D1 rows read for a page returning one row (#1834). rows-read is the billed/throttled unit on D1.The root cause is structural, not a planner-hinting problem: the pivot lacks the columns needed to filter and order without bouncing back to
ec_*. Theec_*composite indexes name exactly which columns those are.Approach
Denormalize the filter + sort columns from
ec_*ontocontent_taxonomies, mirrorec_*'s composite sort indexes onto the pivot keyed by(taxonomy_id, collection, …), and restructure 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 final page.Why this needs no runtime cost model (unlike #1852): once the sort column lives in the pivot index in sort order,
LIMITshort-circuits for a selective term (few rows exist) and a broad term (the limit fills immediately) alike. The index does the work the closed PR tried to do with cached selectivity counts.Decisions
published_at,created_at(loader default),updated_at.jsoncolumn, so the oldEXISTS-vs-DISTINCTPostgres constraint no longer forces divergence).EXISTSper extra taxonomy).deleted_atdeleted_atpredicate and status condition soContentRepositorycan adopt it if admin gains taxonomy filtering. Wiring admin itself is out of scope (it has no taxonomy filter today).ContentRepository,TaxonomyRepository). No stray SQL.Part 1 — Schema: denormalize onto
content_taxonomiesAdd seven columns, all nullable (backfilled by the migration, stamped going forward by Part 4). These are authoritative for which rows appear and in what order, so they must equal the
ec_*row exactly at query time — synchronous re-stamp, not eventual (unlike_emdash_media_usage.content_status, which is maintained by an async snapshot path).statusscheduled_atstatus='scheduled' AND scheduled_at <= nowdeleted_atdeleted_at IS NULL(public/admin-live) orIS NOT NULL(admin trash) — index key columnlocalelocale = ?filterpublished_atcreated_atupdated_atContentTaxonomyTableinpackages/core/src/database/types.tsgains these fields.Part 2 — Indexes: mirror
ec_*'s sort indexes onto the pivotSix composite indexes. Seek prefix is
(taxonomy_id, collection)— equality on both leading columns, so a term shared across collections reads only the queried collection's slice — thendeleted_at, then (for the locale variants)locale, then the sort column DESC, thenentry_idas tiebreaker. This mirrorsec_*'s(deleted_at, [locale,] <sort> DESC, id)shape with the pivot seek prefix prepended.deleted_atis a key column, not a partial predicate. SQLite (and Postgres) can seek an indexed column for bothIS NULLandIS NOT NULL, so one index set serves the public/admin live path (deleted_at IS NULL→ equality-style seek on NULL, clean early-LIMIT) and the admin trash path (deleted_at IS NOT NULL→ a bounded range: still seeks the term, temp-sorts the small trashed candidate set, no early-LIMIT but far better than a scan). A partialWHERE deleted_at IS NULLindex would exclude trash entirely and lock these indexes to the public surface — which is why we keep the column in the key.localefilter is optional (loader.ts:928,AND locale = ?only when a locale is passed). A locale-filtered query wantslocaleas an equality prefix before the sort column; a no-locale query needs the sort column immediately afterdeleted_at. Neither index serves both.status/scheduled_atstay residual (not in the index): the status condition is anOR, so it can't sit before the sort column without breaking sort order. The denormalized columns make it a pivot-local read, not anec_*bounce, and — because status is a filter column rather than an index constraint — the same indexes serve the publicpublished-or-scheduledcondition and an admin all-statuses / draft-only condition. This mirrorsec_*exactly (it keeps a separate(deleted_at, status)index and excludesstatusfrom the sort indexes).ec_*'s migration 041 added locale variants forupdated_at/created_atbut notpublished_at, because it was scoped to the admin i18n content list (which sorts by edited/created). For taxonomy listings the dominant real-world shape is a public category page filtered to a locale and sorted bypublished_at, soloc_pubis included and is the most-used locale variant — the opposite ofec_*'s omission.(collection, entry_id, taxonomy_id),idx_content_taxonomies_term (taxonomy_id)from migration 048). The new composites lead withtaxonomy_idand arguably supersede 048, but dropping it is out of scope (additive-only discipline; 048 was a deliberate restore for Migration 036 can permanently lose idx_content_taxonomies_term on a partial-apply retry (same class as #1665) #1701).Part 3 — Query restructure (
packages/core/src/loader.ts)Drive from a pivot-only CTE that carries the sort column, then join
ec_*by PK for hydration:Single group (the reported bug + most traffic): no
GROUP BY; the pivot index is covering for(entry_id, sortval)and gives clean early-LIMIT. Then a bounded PK join toec_*.Multiple groups (OR-within-a-taxonomy, ≥2 slugs): an entry tagged with two matched groups yields two pivot rows → fan-out.
GROUP BY ct.entry_idrestores semi-join semantics (one row per entry), withMAX(sortval)as the representative sort key. This gives up clean early-LIMIT (it aggregates all matching pivot rows before limiting) but stays bounded to tagged rows — still far cheaper than the full-collection scan. Because the driving CTE selects only scalar pivot columns, noDISTINCT/GROUP BYever touches ajsoncolumn, so both dialects run the identical shape — this is what makes "unify dialects" safe rather than risky (it sidesteps the originalEXISTS-vs-DISTINCTPostgres constraint documented atloader.ts:917-922).Multi-term AND (an entry must match a term in every requested taxonomy): drive
pickedfrom the first taxonomy's groups; each additional taxonomy becomes a residual clause insidepicked'sWHERE:This hits the PK
(collection, entry_id, taxonomy_id)— a cheap point lookup per candidate.Cursor / pagination.
buildCursorConditionkeys on(sortval, entry_id)from the pivot CTE.LIMIT/OFFSETapply topicked.Non-date
orderBy. WhenorderByis an arbitraryec_*field (not one of the three denormalized date columns), the fast path still seeks the term via the pivot but sorts the joined candidate set (no pivot sort index applies). Bounded to tagged rows — worse than early-LIMIT for a broad term, but there is no index for arbitrary-field sorts anywhere (unfiltered listings temp-sort them too), and field-sorted taxonomy pages are rare.Byline filters are untouched — they keep their existing
EXISTSagainst_emdash_content_bylines(loader.ts:959-966); only the taxonomy path is restructured.Caller-agnostic builder (admin reuse). Factor the
picked-CTE construction into a helper parameterized on: the resolved group set, the sort column, thedeleted_atpredicate (IS NULL/IS NOT NULL), the status condition, the optionallocale, and the cursor/limit. The public loader passes its published-or-scheduled +deleted_at IS NULLshape;ContentRepositorycan pass an all-statuses +deleted_at IS NOT NULL(trash) orIS NULL(live) shape without any schema change — the columns (Part 1) and indexes (Part 2) already cover it. Actually wiring admin's content list to filter by taxonomy is out of scope (it has no taxonomy filter today,content.ts:510-529); this change only ensures the substrate doesn't preclude it.Part 4 — Write path: re-stamp in the data layer
The denormalized columns must equal the
ec_*row synchronously on every mutation that moves them.ContentRepository(packages/core/src/database/repositories/content.ts) —update,delete(soft),restore,schedule,unschedule, and the scheduled-publish cron: after mutating theec_*row, re-stamp every pivot row for that entry:UPDATE content_taxonomies SET status, scheduled_at, deleted_at, published_at, created_at, updated_at, locale = <new> WHERE collection = ? AND entry_id = ?.TaxonomyRepository(packages/core/src/database/repositories/taxonomy.ts) —attachToEntry,setTermsForEntry,copyEntryTerms: stamp the entry's currentec_*values into the new pivot rows at insert time.Both are existing choke points; no state-changing SQL for
content_taxonomieslives outside these repositories.Part 5 — Migration
051_content_taxonomies_denorm.ts(forward-only)ALTER TABLE content_taxonomies ADD COLUMN …× 7 (Part 1)._emdash_collections(migration 013's multi-table pattern): per collection slug,UPDATE content_taxonomies SET <cols> = (SELECT <cols> FROM ec_<slug> WHERE id = content_taxonomies.entry_id) WHERE collection = ?. Validate the slug (sql.ref) — never interpolate. New rows are stamped by Part 4 going forward.Register in
runner.ts(static import +getMigrations()).Part 6 — Testing (TDD,
describeEachDialect)The invariant: the seek returns byte-identical rows to the old
EXISTS.draftquery returns drafts, apublishedquery returns published). Both dialects.deleted_at IS NOT NULL(trash) and an all-statuses condition, exercising thedeleted_at-in-key path. (No admin route consumes this yet; the test targets the helper directly to keep the substrate honest.)EXPLAIN QUERY PLAN: a single-term query (selective and broad) uses a pivot sort index withLIMITshort-circuit and does not full-scanec_*.ec_*; a status change flips which rows the listing returns.cursorover the pivot sort key returns correctly-ordered, non-overlapping pages.pnpm query-countssnapshot reviewed; expected delta is the taxonomy-filtered routes only.Cost & implications
Six composite indexes on the pivot is not free. Being explicit so a reviewer doesn't have to reverse-engineer the tradeoff:
updated_atis the dominant write cost.ContentRepository.updatestampsupdated_at = nowon every save (content.ts:595). Becauseupdated_atis a sort key, each save re-stamps the entry's pivot rows and moves them withinidx_content_taxonomies_updand_loc_upd— ~2 index moves per pivot row per save (for an entry with K terms, ~2K).published_at/created_atchange rarely, so their four indexes churn far less. Of the six, the twoupdated_atindexes carry the highest write cost for the lowest read value (updated_at is an admin-oriented sort, seldom a public category-page sort). They are included per the "all three sorts / not half-baked" decision; if write cost on edit-heavy sites proves painful, droppingupd/loc_upd(falling back to temp-sort of the bounded candidate set) is the first lever.content_taxonomiesinsert (WXR importwxr-taxonomies.ts,seed/apply.ts,copyEntryTermson translation create) now maintains six b-trees. On D1, index maintenance counts toward rows-written (billed/throttled), so large imports get measurably heavier. This is the write-side cost of the read-side win.deleted_atin the key (vs. a partial index) adds little: trashed rows stay indexed (small size bump) and soft-delete/restore moves rows rather than removing/re-adding them. This is the price of serving the admin trash path from the same indexes — an intentional trade, not overhead.taxonomy_id/entry_id≈ 26 chars, ISO timestamps ≈ 24 chars) over the full pivot — a few MB per large collection's worth. Real on D1's size budget, not alarming.ORDER BYtail match), so it is deterministic — but the Part 6EXPLAIN QUERY PLANtests must assert the right index per case (locale-filtered +published_at→loc_pub, notpubwith a residual locale filter).deleted_at IS NOT NULLis a range, so the sort column after it isn't globally ordered → SQLite temp-sorts the (bounded) trashed candidate set. Live content keeps the cleanLIMITshort-circuit. Acceptable given trash volume and traffic.content_taxonomiesis O(n log n) ×6. Build them after the backfillUPDATE; on a big D1 the051migration runs longer than the schema change alone implies.Follow-up (out of scope, flag for the PR)
idx_content_taxonomies_term (taxonomy_id)(migration 048) becomes redundant — all six new indexes lead withtaxonomy_idand serve a baretaxonomy_idseek as a prefix. Keeping it means a seventh index churned on writes for no read benefit. Dropping it is a separate, additive-discipline scope call (048 was a deliberate restore for #1701), but it should be raised so the write cost isn't paid twice.Backwards compatibility & changeset
Additive: new nullable pivot columns, new indexes, a forward-only backfill migration. No API surface change; identical result rows (seek and scan return the same rows — tested). Query results and ordering are unchanged; only the access path and D1 rows-read differ.
Changeset (present-tense, user-facing):
All reactions