Skip to content

Commit 5ed84a2

Browse files
MA2153claudeemdashbot[bot]
authored andcommitted
fix(loader): seek taxonomy-filtered listings via a denormalized pivot (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>
1 parent ceae83c commit 5ed84a2

17 files changed

Lines changed: 1234 additions & 50 deletions
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"emdash": patch
3+
---
4+
5+
Fixes taxonomy-filtered collection listings reading the entire collection on SQLite/D1 when the term is selective. Such listings now seek the matching entries directly, cutting D1 rows-read from tens of thousands to the page size.
Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
import type { Kysely } from "kysely";
2+
import { sql } from "kysely";
3+
4+
import { columnExists, listTablesLike } from "../dialect-helpers.js";
5+
import { validateIdentifier } from "../validate.js";
6+
7+
/**
8+
* Denormalize the filter + sort columns from `ec_*` onto `content_taxonomies`
9+
* and mirror `ec_*`'s composite sort indexes onto the pivot (#1834).
10+
*
11+
* A taxonomy-filtered listing used to apply the term filter as a correlated
12+
* `EXISTS` on `SELECT * FROM ec_<collection> … ORDER BY … LIMIT ?`. The pivot
13+
* carried only `(collection, entry_id, taxonomy_id)`, so the filter
14+
* (`deleted_at`, `status`, `locale`) and the sort (`published_at`/`created_at`)
15+
* could only be evaluated on the `ec_*` row. On stats-blind SQLite/D1 a
16+
* selective term never fills the `LIMIT`, so the query walked the whole
17+
* collection (~75k D1 rows read for a one-row page).
18+
*
19+
* Copying those columns onto the pivot and indexing them keyed by
20+
* `(taxonomy_id, collection, …)` lets the loader seek the term and walk a
21+
* sort-ordered pivot index, short-circuiting on `LIMIT`, then touch `ec_*` only
22+
* by primary key to hydrate the page.
23+
*
24+
* The columns are advisory (D1 has no transactions; the write-path re-stamp is
25+
* non-atomic), so the read path re-checks the real predicates on the joined
26+
* `ec_*` row — see loader.ts. `updated_at` is deliberately not denormalized: it
27+
* moves on every edit and is seldom a public sort, so its write cost outweighs
28+
* its read value; `updated_at`-sorted taxonomy listings temp-sort instead.
29+
*
30+
* Order matters: add the columns, backfill, THEN build the indexes so each
31+
* index is built once over populated data rather than maintained through the
32+
* backfill `UPDATE`.
33+
*
34+
* Forward-only.
35+
*/
36+
37+
const DENORM_COLUMNS = [
38+
"status",
39+
"scheduled_at",
40+
"deleted_at",
41+
"locale",
42+
"published_at",
43+
"created_at",
44+
] as const;
45+
46+
// `entry_id DESC` (not ASC) matters: the listing orders `<sort> DESC, entry_id
47+
// DESC` (the common case), and a DESC tiebreaker lets SQLite satisfy the whole
48+
// ORDER BY from the index — no temp B-tree, clean early-`LIMIT`. An ASC
49+
// tiebreaker forces `USE TEMP B-TREE FOR LAST TERM OF ORDER BY`, which buffers
50+
// a whole equal-`sortval` block (e.g. a bulk import sharing one timestamp)
51+
// before emitting. `DESC` also serves the rarer ASC listing via a backward
52+
// index scan. Mirrors `ec_*`'s `(deleted_at, [locale,] <sort> DESC, id DESC)`.
53+
const INDEXES: { name: string; columns: string }[] = [
54+
{
55+
name: "idx_content_taxonomies_pub",
56+
columns: "taxonomy_id, collection, deleted_at, published_at DESC, entry_id DESC",
57+
},
58+
{
59+
name: "idx_content_taxonomies_crt",
60+
columns: "taxonomy_id, collection, deleted_at, created_at DESC, entry_id DESC",
61+
},
62+
{
63+
name: "idx_content_taxonomies_loc_pub",
64+
columns: "taxonomy_id, collection, deleted_at, locale, published_at DESC, entry_id DESC",
65+
},
66+
{
67+
name: "idx_content_taxonomies_loc_crt",
68+
columns: "taxonomy_id, collection, deleted_at, locale, created_at DESC, entry_id DESC",
69+
},
70+
];
71+
72+
export async function up(db: Kysely<unknown>): Promise<void> {
73+
// 1. Add the six denormalized columns (all nullable, no default). Guarded so
74+
// a partial apply — or a re-run after a mid-migration failure — can retry
75+
// (`ALTER TABLE ADD COLUMN` is not itself idempotent on either dialect).
76+
for (const column of DENORM_COLUMNS) {
77+
if (await columnExists(db, "content_taxonomies", column)) continue;
78+
await sql`
79+
ALTER TABLE content_taxonomies ADD COLUMN ${sql.ref(column)} TEXT
80+
`.execute(db);
81+
}
82+
83+
// 2. Backfill from each content table by primary key. Drive from the actual
84+
// `ec_*` tables (not `_emdash_collections`) so collections whose table was
85+
// dropped are skipped naturally; the pivot's `collection` value is the
86+
// slug, i.e. the table name without its `ec_` prefix.
87+
const tableNames = await listTablesLike(db, "ec_%");
88+
for (const tableName of tableNames) {
89+
validateIdentifier(tableName, "content table name");
90+
const slug = tableName.slice("ec_".length);
91+
await sql`
92+
UPDATE content_taxonomies
93+
SET (status, scheduled_at, deleted_at, locale, published_at, created_at) = (
94+
SELECT status, scheduled_at, deleted_at, locale, published_at, created_at
95+
FROM ${sql.ref(tableName)}
96+
WHERE ${sql.ref(tableName)}.id = content_taxonomies.entry_id
97+
)
98+
WHERE collection = ${slug}
99+
`.execute(db);
100+
}
101+
102+
// 3. Build the composite sort indexes over the now-populated columns.
103+
for (const index of INDEXES) {
104+
await sql`
105+
CREATE INDEX IF NOT EXISTS ${sql.ref(index.name)}
106+
ON content_taxonomies (${sql.raw(index.columns)})
107+
`.execute(db);
108+
}
109+
}
110+
111+
export async function down(db: Kysely<unknown>): Promise<void> {
112+
for (const index of INDEXES) {
113+
await sql`DROP INDEX IF EXISTS ${sql.ref(index.name)}`.execute(db);
114+
}
115+
for (const column of DENORM_COLUMNS) {
116+
await sql`
117+
ALTER TABLE content_taxonomies DROP COLUMN ${sql.ref(column)}
118+
`.execute(db);
119+
}
120+
}

packages/core/src/database/migrations/runner.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ import * as m047 from "./047_restore_taxonomy_parent_index.js";
5353
import * as m048 from "./048_restore_content_taxonomies_term_index.js";
5454
import * as m049 from "./049_taxonomies_name_locale_index.js";
5555
import * as m050 from "./050_media_usage_index_status.js";
56+
import * as m051 from "./051_content_taxonomies_denorm.js";
5657

5758
const MIGRATIONS: Readonly<Record<string, Migration>> = Object.freeze({
5859
"001_initial": m001,
@@ -104,6 +105,7 @@ const MIGRATIONS: Readonly<Record<string, Migration>> = Object.freeze({
104105
"048_restore_content_taxonomies_term_index": m048,
105106
"049_taxonomies_name_locale_index": m049,
106107
"050_media_usage_index_status": m050,
108+
"051_content_taxonomies_denorm": m051,
107109
});
108110

109111
/** Total number of registered migrations. Exported for use in tests. */

packages/core/src/database/repositories/content.ts

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -637,6 +637,18 @@ export class ContentRepository {
637637
.where("deleted_at" as never, "is", null)
638638
.execute();
639639

640+
// Re-stamp the taxonomy pivot only when a denormalized column actually
641+
// moved (status/publishedAt/scheduledAt). A plain content edit bumps
642+
// `updated_at` — which is not denormalized — so it needs no pivot write,
643+
// keeping the common edit path free of taxonomy write amplification.
644+
if (
645+
input.status !== undefined ||
646+
input.publishedAt !== undefined ||
647+
input.scheduledAt !== undefined
648+
) {
649+
await this.restampEntryPivot(type, id);
650+
}
651+
640652
invalidateCollectionCache(type);
641653

642654
const updated = await this.findById(type, id);
@@ -662,7 +674,10 @@ export class ContentRepository {
662674
`.execute(this.db);
663675

664676
const changed = (result.numAffectedRows ?? 0n) > 0n;
665-
if (changed) invalidateCollectionCache(type);
677+
if (changed) {
678+
await this.restampEntryPivot(type, id);
679+
invalidateCollectionCache(type);
680+
}
666681
return changed;
667682
}
668683

@@ -683,10 +698,36 @@ export class ContentRepository {
683698
const restored = result.rows[0];
684699
if (!restored) return null;
685700

701+
await this.restampEntryPivot(type, id);
686702
invalidateCollectionCache(type);
687703
return this.mapRow(type, restored);
688704
}
689705

706+
/**
707+
* Re-stamp the denormalized filter + sort columns on every
708+
* `content_taxonomies` pivot row for an entry from its authoritative `ec_*`
709+
* row (migration 051). Called after any mutation that moves one of those
710+
* columns so a taxonomy-filtered listing can seek the entry directly.
711+
*
712+
* A single correlated `UPDATE` reads the post-mutation values from `ec_*`, so
713+
* the pivot converges to the authoritative row. This is NOT atomic with the
714+
* `ec_*` mutation on D1 (no transactions), which is why the read path
715+
* re-checks the real predicates on the joined `ec_*` row. Untagged entries
716+
* have no pivot rows, so the statement is a cheap no-op for them.
717+
*/
718+
private async restampEntryPivot(type: string, id: string): Promise<void> {
719+
const tableName = getTableName(type);
720+
await sql`
721+
UPDATE content_taxonomies
722+
SET (status, scheduled_at, deleted_at, locale, published_at, created_at) = (
723+
SELECT status, scheduled_at, deleted_at, locale, published_at, created_at
724+
FROM ${sql.ref(tableName)}
725+
WHERE ${sql.ref(tableName)}.id = ${id}
726+
)
727+
WHERE collection = ${type} AND entry_id = ${id}
728+
`.execute(this.db);
729+
}
730+
690731
/**
691732
* Permanently delete content (cannot be undone)
692733
*/
@@ -996,6 +1037,7 @@ export class ContentRepository {
9961037
AND deleted_at IS NULL
9971038
`.execute(this.db);
9981039

1040+
await this.restampEntryPivot(type, id);
9991041
invalidateCollectionCache(type);
10001042

10011043
const updated = await this.findById(type, id);
@@ -1035,6 +1077,7 @@ export class ContentRepository {
10351077
AND deleted_at IS NULL
10361078
`.execute(this.db);
10371079

1080+
await this.restampEntryPivot(type, id);
10381081
invalidateCollectionCache(type);
10391082

10401083
const updated = await this.findById(type, id);
@@ -1339,6 +1382,8 @@ export class ContentRepository {
13391382
}
13401383
publishCommitted = true;
13411384

1385+
await this.restampEntryPivot(type, id);
1386+
13421387
const updated = await this.findById(type, id);
13431388
if (!updated) {
13441389
throw new Error("Content not found");
@@ -1427,6 +1472,7 @@ export class ContentRepository {
14271472
AND deleted_at IS NULL
14281473
`.execute(this.db);
14291474

1475+
await this.restampEntryPivot(type, id);
14301476
invalidateCollectionCache(type);
14311477

14321478
const updated = await this.findById(type, id);

packages/core/src/database/repositories/taxonomy.ts

Lines changed: 57 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,33 @@
1-
import type { Kysely, Selectable } from "kysely";
1+
import { sql, type Kysely, type Selectable } from "kysely";
22
import { ulid } from "ulidx";
33

44
import { invalidateTaxonomyObjectCache } from "../../object-cache/index.js";
5-
import type { Database, TaxonomyTable, ContentTaxonomyTable } from "../types.js";
5+
import { isMissingTableError } from "../../utils/db-errors.js";
6+
import type { Database, TaxonomyTable } from "../types.js";
7+
import { validateIdentifier } from "../validate.js";
8+
9+
/**
10+
* Filter + sort columns denormalized from an entry's `ec_*` row onto its
11+
* `content_taxonomies` pivot rows (migration 051). Stamped at insert time so a
12+
* newly-tagged entry is immediately seekable by a taxonomy-filtered listing.
13+
*/
14+
interface PivotDenorm {
15+
status: string | null;
16+
scheduled_at: string | null;
17+
deleted_at: string | null;
18+
locale: string | null;
19+
published_at: string | null;
20+
created_at: string | null;
21+
}
22+
23+
const EMPTY_DENORM: PivotDenorm = {
24+
status: null,
25+
scheduled_at: null,
26+
deleted_at: null,
27+
locale: null,
28+
published_at: null,
29+
created_at: null,
30+
};
631

732
export interface Taxonomy {
833
id: string;
@@ -251,14 +276,10 @@ export class TaxonomyRepository {
251276
const group = await this.resolveTranslationGroup(taxonomyId);
252277
if (!group) return;
253278

254-
const row: ContentTaxonomyTable = {
255-
collection,
256-
entry_id: entryId,
257-
taxonomy_id: group,
258-
};
279+
const denorm = await this.fetchEntryDenorm(collection, entryId);
259280
await this.db
260281
.insertInto("content_taxonomies")
261-
.values(row)
282+
.values({ collection, entry_id: entryId, taxonomy_id: group, ...denorm })
262283
.onConflict((oc) => oc.doNothing())
263284
.execute();
264285
invalidateTaxonomyObjectCache();
@@ -342,13 +363,15 @@ export class TaxonomyRepository {
342363

343364
const toAdd = [...newGroups].filter((g) => !currentGroups.has(g));
344365
if (toAdd.length > 0) {
366+
const denorm = await this.fetchEntryDenorm(collection, entryId);
345367
await this.db
346368
.insertInto("content_taxonomies")
347369
.values(
348370
toAdd.map((taxonomy_id) => ({
349371
collection,
350372
entry_id: entryId,
351373
taxonomy_id,
374+
...denorm,
352375
})),
353376
)
354377
.onConflict((oc) => oc.doNothing())
@@ -387,20 +410,46 @@ export class TaxonomyRepository {
387410
.execute();
388411
if (rows.length === 0) return;
389412

413+
// Stamp the TARGET entry's current values — the copy inherits the source's
414+
// term memberships but the target's own status/dates/locale.
415+
const denorm = await this.fetchEntryDenorm(collection, targetEntryId);
390416
await this.db
391417
.insertInto("content_taxonomies")
392418
.values(
393419
rows.map((r) => ({
394420
collection,
395421
entry_id: targetEntryId,
396422
taxonomy_id: r.taxonomy_id,
423+
...denorm,
397424
})),
398425
)
399426
.onConflict((oc) => oc.doNothing())
400427
.execute();
401428
invalidateTaxonomyObjectCache();
402429
}
403430

431+
/**
432+
* Read the denormalized filter + sort columns from an entry's `ec_*` row so
433+
* they can be stamped onto new pivot rows (migration 051). A missing table or
434+
* missing row yields all-nulls: the pivot columns are advisory, and the
435+
* listing read path re-checks the authoritative `ec_*` row regardless.
436+
*/
437+
private async fetchEntryDenorm(collection: string, entryId: string): Promise<PivotDenorm> {
438+
validateIdentifier(collection, "collection type");
439+
const tableName = `ec_${collection}`;
440+
try {
441+
const result = await sql<PivotDenorm>`
442+
SELECT status, scheduled_at, deleted_at, locale, published_at, created_at
443+
FROM ${sql.ref(tableName)}
444+
WHERE id = ${entryId}
445+
`.execute(this.db);
446+
return result.rows[0] ?? EMPTY_DENORM;
447+
} catch (error) {
448+
if (isMissingTableError(error)) return EMPTY_DENORM;
449+
throw error;
450+
}
451+
}
452+
404453
/**
405454
* Count content entries that use any translation of this term. Accepts
406455
* either a term id or a translation_group — we normalise to the group.

packages/core/src/database/types.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,27 @@ export interface ContentTaxonomyTable {
2828
collection: string; // e.g., 'posts'
2929
entry_id: string; // ID in the ec_* table
3030
taxonomy_id: string; // stores taxonomies.translation_group (locale-agnostic)
31+
// Denormalized filter + sort columns mirrored from the entry's ec_* row
32+
// (migration 051). They let a taxonomy-filtered listing seek the matching
33+
// entries directly on the pivot instead of scanning the whole collection.
34+
//
35+
// ADVISORY, not authoritative: D1 has no transactions, so the write-path
36+
// re-stamp (ContentRepository / TaxonomyRepository) is a separate statement
37+
// from the ec_* mutation and can be transiently stale. The read path narrows
38+
// the candidate set with these columns but re-checks the real filter
39+
// predicates on the joined ec_* row. `updated_at` is deliberately NOT
40+
// denormalized — it moves on every edit, so denormalizing it would force a
41+
// pivot re-stamp on the common edit path for little read value.
42+
//
43+
// Nullable/Generated so inserts that predate a re-stamp (or the migration
44+
// backfill) leave them NULL; every insert site in the repositories stamps
45+
// them explicitly.
46+
status: Generated<string | null>;
47+
scheduled_at: Generated<string | null>;
48+
deleted_at: Generated<string | null>;
49+
locale: Generated<string | null>;
50+
published_at: Generated<string | null>;
51+
created_at: Generated<string | null>;
3152
}
3253

3354
export interface TaxonomyDefTable {

0 commit comments

Comments
 (0)