diff --git a/.changeset/translation-group-locale-index.md b/.changeset/translation-group-locale-index.md new file mode 100644 index 0000000000..147db0e076 --- /dev/null +++ b/.changeset/translation-group-locale-index.md @@ -0,0 +1,5 @@ +--- +"emdash": patch +--- + +Fixes translation lookups reading every non-deleted row of a content table. Content tables now carry `(translation_group, locale)` and `(deleted_at, translation_group, locale)` indexes, replacing the single-column `translation_group` one, so fetching an entry's translations — one entry's, a whole page's, or a menu link's target — seeks straight to the group instead of scanning. The improvement is largest on big collections and on D1, where the query planner has no statistics to fall back on. diff --git a/packages/core/src/database/migrations/055_content_translation_group_locale_index.ts b/packages/core/src/database/migrations/055_content_translation_group_locale_index.ts new file mode 100644 index 0000000000..2c08648a37 --- /dev/null +++ b/packages/core/src/database/migrations/055_content_translation_group_locale_index.ts @@ -0,0 +1,67 @@ +import type { Kysely } from "kysely"; +import { sql } from "kysely"; + +import { listTablesLike } from "../dialect-helpers.js"; + +/** + * Migration: replace the single-column content `translation_group` index with + * one index per translation-group read shape. + * + * There are two shapes. Translation-group reads filter `deleted_at IS NULL` + * with `translation_group = ?` / `IN (...)` and order by locale; menu and + * reference resolution looks a group up by `translation_group` (+ `locale`) + * alone, with no `deleted_at` term. + * + * Neither shape can borrow the other's index. Seeking migration 041's + * `(deleted_at, locale, ...)` composites on `deleted_at` alone already returns + * rows in locale order, so a stats-blind planner prefers them over a + * single-column `translation_group` index and reads every non-deleted row in + * the table; the batched variant is worse still, because the `IN (...)` list + * multiplies the planner's row estimate for a `translation_group`-leading index + * and it falls back to a `deleted_at` composite from a handful of groups + * onward. Going the other way, an index leading with `deleted_at` cannot seek a + * lookup that never constrains that column. D1 never has `sqlite_stat1`, so the + * index shape is the only lever and each shape needs a prefix matching it term + * for term. + * + * Forward-only and idempotent (`IF NOT EXISTS`). + * + * Index names use short `tg_locale` / `del_tg_locale` suffixes rather than + * spelling out `translation_group`: Postgres truncates identifiers to 63 bytes, + * and the longer forms truncate to the same value for long collection slugs. + * Keep these identical to the names in `schema/registry.ts`. + */ +export async function up(db: Kysely): Promise { + const tableNames = await listTablesLike(db, "ec_%"); + + for (const tableName of tableNames) { + // D1 DDL is non-transactional: create the replacements before dropping the + // old index so an interrupted migration always leaves a + // translation_group-leading index in place. + await sql` + CREATE INDEX IF NOT EXISTS ${sql.ref(`idx_${tableName}_tg_locale`)} + ON ${sql.ref(tableName)} (translation_group, locale) + `.execute(db); + + await sql` + CREATE INDEX IF NOT EXISTS ${sql.ref(`idx_${tableName}_del_tg_locale`)} + ON ${sql.ref(tableName)} (deleted_at, translation_group, locale) + `.execute(db); + + await sql`DROP INDEX IF EXISTS ${sql.ref(`idx_${tableName}_translation_group`)}`.execute(db); + } +} + +export async function down(db: Kysely): Promise { + const tableNames = await listTablesLike(db, "ec_%"); + + for (const tableName of tableNames) { + await sql` + CREATE INDEX IF NOT EXISTS ${sql.ref(`idx_${tableName}_translation_group`)} + ON ${sql.ref(tableName)} (translation_group) + `.execute(db); + + await sql`DROP INDEX IF EXISTS ${sql.ref(`idx_${tableName}_del_tg_locale`)}`.execute(db); + await sql`DROP INDEX IF EXISTS ${sql.ref(`idx_${tableName}_tg_locale`)}`.execute(db); + } +} diff --git a/packages/core/src/database/migrations/runner.ts b/packages/core/src/database/migrations/runner.ts index 5c1c7764ff..33b8eecfde 100644 --- a/packages/core/src/database/migrations/runner.ts +++ b/packages/core/src/database/migrations/runner.ts @@ -57,6 +57,7 @@ import * as m051 from "./051_content_taxonomies_denorm.js"; import * as m052 from "./052_media_usage_read_index.js"; import * as m053 from "./053_plugin_mcp_tools.js"; import * as m054 from "./054_media_upload_attempts.js"; +import * as m055 from "./055_content_translation_group_locale_index.js"; const MIGRATIONS: Readonly> = Object.freeze({ "001_initial": m001, @@ -112,6 +113,7 @@ const MIGRATIONS: Readonly> = Object.freeze({ "052_media_usage_read_index": m052, "053_plugin_mcp_tools": m053, "054_media_upload_attempts": m054, + "055_content_translation_group_locale_index": m055, }); /** Total number of registered migrations. Exported for use in tests. */ diff --git a/packages/core/src/database/repositories/content.ts b/packages/core/src/database/repositories/content.ts index 62750c8dfe..5095564eb1 100644 --- a/packages/core/src/database/repositories/content.ts +++ b/packages/core/src/database/repositories/content.ts @@ -1158,6 +1158,10 @@ export class ContentRepository { * Lets callers resolve many edge groups without an N+1 per group. The caller * groups the flat result by `translationGroup` itself. * + * `translation_group` leads the sort so the ordering follows + * `idx_{table}_del_tg_locale` past its `deleted_at` equality; callers group by + * `translationGroup`, so the per-group locale order they rely on is preserved. + * * `publishedOnly` restricts the result to `status = 'published'` — reference * reads pass this for callers without `content:read_drafts` so draft/scheduled * entries never leak through an edge traversal. @@ -1184,7 +1188,7 @@ export class ContentRepository { WHERE translation_group IN (${sql.join(chunk)}) AND deleted_at IS NULL ${publishedFilter} - ORDER BY locale ASC + ORDER BY translation_group ASC, locale ASC `.execute(this.db); for (const row of result.rows) items.push(this.mapRow(type, row)); } diff --git a/packages/core/src/schema/registry.ts b/packages/core/src/schema/registry.ts index a2608f45a2..f8f977dd34 100644 --- a/packages/core/src/schema/registry.ts +++ b/packages/core/src/schema/registry.ts @@ -984,9 +984,18 @@ export class SchemaRegistry { ON ${sql.ref(tableName)} (locale) `.execute(conn); + // Names must stay identical to migration 055, which creates these indexes + // on tables that already exist. Lookups that don't constrain `deleted_at` + // (menu and reference resolution) need the first; reads that do need the + // second. await sql` - CREATE INDEX ${sql.ref(`idx_${tableName}_translation_group`)} - ON ${sql.ref(tableName)} (translation_group) + CREATE INDEX ${sql.ref(`idx_${tableName}_tg_locale`)} + ON ${sql.ref(tableName)} (translation_group, locale) + `.execute(conn); + + await sql` + CREATE INDEX ${sql.ref(`idx_${tableName}_del_tg_locale`)} + ON ${sql.ref(tableName)} (deleted_at, translation_group, locale) `.execute(conn); // Composite indexes for optimized query performance (see migration 033) diff --git a/packages/core/tests/integration/database/content-translation-index-name.test.ts b/packages/core/tests/integration/database/content-translation-index-name.test.ts new file mode 100644 index 0000000000..1ee91f584c --- /dev/null +++ b/packages/core/tests/integration/database/content-translation-index-name.test.ts @@ -0,0 +1,80 @@ +import { sql } from "kysely"; +import { afterEach, beforeEach, expect, it } from "vitest"; + +import * as migration055 from "../../../src/database/migrations/055_content_translation_group_locale_index.js"; +import { SchemaRegistry } from "../../../src/schema/registry.js"; +import { + type DialectTestContext, + describeEachDialect, + setupForDialect, + teardownForDialect, +} from "../../utils/test-db.js"; + +/** + * The longest collection slug `SchemaRegistry` can create on Postgres: at 47 + * characters the `deleted_updated_id` and `deleted_status` index names collide + * once Postgres truncates identifiers to 63 bytes. `idx_{table}_tg_locale` and + * `idx_{table}_del_tg_locale` are truncated at this length too, so the + * migration's creates and drop must still name three different indexes. + */ +const LONG_SLUG = `t${"o".repeat(45)}`; +const TABLE_NAME = `ec_${LONG_SLUG}`; + +describeEachDialect("translation_group index replacement for long collection slugs", (dialect) => { + let ctx: DialectTestContext; + + beforeEach(async () => { + ctx = await setupForDialect(dialect); + const registry = new SchemaRegistry(ctx.db); + await registry.createCollection({ slug: LONG_SLUG, label: "Long", labelSingular: "Long" }); + + await sql`DROP INDEX IF EXISTS ${sql.ref(`idx_${TABLE_NAME}_tg_locale`)}`.execute(ctx.db); + await sql`DROP INDEX IF EXISTS ${sql.ref(`idx_${TABLE_NAME}_del_tg_locale`)}`.execute(ctx.db); + await sql` + CREATE INDEX ${sql.ref(`idx_${TABLE_NAME}_translation_group`)} + ON ${sql.ref(TABLE_NAME)} (translation_group) + `.execute(ctx.db); + }); + + afterEach(async () => { + await teardownForDialect(ctx); + }); + + it("leaves the table with both composite indexes, not with one or none", async () => { + await migration055.up(ctx.db); + + const covering = (await translationIndexColumns()) + .filter((columns) => columns.includes("translation_group")) + .toSorted(); + expect(covering).toEqual([ + "deleted_at, translation_group, locale", + "translation_group, locale", + ]); + }); + + async function translationIndexColumns(): Promise { + if (ctx.dialect === "postgres") { + const result = await sql<{ indexdef: string }>` + SELECT indexdef FROM pg_indexes + WHERE schemaname = current_schema() AND tablename = ${TABLE_NAME} + `.execute(ctx.db); + return result.rows.map((row) => columnList(row.indexdef)); + } + + const result = await sql<{ sql: string | null }>` + SELECT sql FROM sqlite_master WHERE type = 'index' AND tbl_name = ${TABLE_NAME} + `.execute(ctx.db); + return result.rows.map((row) => columnList(row.sql ?? "")); + } +}); + +function columnList(definition: string): string { + const open = definition.lastIndexOf("("); + const close = definition.lastIndexOf(")"); + if (open === -1 || close < open) return ""; + return definition + .slice(open + 1, close) + .replaceAll('"', "") + .replaceAll(/\s+/g, " ") + .trim(); +} diff --git a/packages/core/tests/integration/database/content-translations-plan.test.ts b/packages/core/tests/integration/database/content-translations-plan.test.ts new file mode 100644 index 0000000000..1e2b0ff163 --- /dev/null +++ b/packages/core/tests/integration/database/content-translations-plan.test.ts @@ -0,0 +1,214 @@ +/** + * Query-plan coverage for translation-group reads on content tables. + * + * SQLite runs without ANALYZE/sqlite_stat1 here, matching D1's stats-blind + * planner. Result parity for these reads is covered by the i18n suite. + */ + +import Database from "better-sqlite3"; +import { Kysely, SqliteDialect } from "kysely"; +import { afterEach, beforeEach, expect, it } from "vitest"; + +import * as migration055 from "../../../src/database/migrations/055_content_translation_group_locale_index.js"; +import { runMigrations } from "../../../src/database/migrations/runner.js"; +import { ContentRepository } from "../../../src/database/repositories/content.js"; +import type { Database as DatabaseSchema } from "../../../src/database/types.js"; +import { getMenuWithDb } from "../../../src/menus/index.js"; +import { SchemaRegistry } from "../../../src/schema/registry.js"; +import { SQL_BATCH_SIZE } from "../../../src/utils/chunks.js"; + +interface CapturedQuery { + sql: string; + parameters: readonly unknown[]; +} + +let sqlite: Database.Database; +let db: Kysely; +let repo: ContentRepository; +let captured: CapturedQuery[]; + +beforeEach(async () => { + captured = []; + sqlite = new Database(":memory:"); + db = new Kysely({ + dialect: new SqliteDialect({ database: sqlite }), + log(event) { + if (event.level === "query") { + captured.push({ sql: event.query.sql, parameters: event.query.parameters }); + } + }, + }); + await runMigrations(db); + const registry = new SchemaRegistry(db); + await registry.createCollection({ slug: "post", label: "Posts", labelSingular: "Post" }); + repo = new ContentRepository(db); + + for (let index = 1; index <= 200; index++) { + const group = `tg-${String(Math.ceil(index / 2)).padStart(4, "0")}`; + sqlite + .prepare( + `INSERT INTO ec_post (id, slug, status, locale, translation_group, created_at, updated_at, version) + VALUES (?, ?, 'published', ?, ?, '2025-01-01', '2025-01-01', 1)`, + ) + .run(`id-${index}`, `slug-${index}`, index % 2 === 0 ? "en" : "de", group); + } + captured = []; +}); + +afterEach(async () => { + await db.destroy(); +}); + +it("seeks a single translation group through the translation_group index", async () => { + const items = await repo.findTranslations("post", "tg-0005"); + + expect(items.map((item) => item.locale)).toEqual(["de", "en"]); + + const query = translationQuery(); + const plan = explain(query); + expect(contentAccess(plan)).toMatch( + /SEARCH ec_post USING (?:COVERING )?INDEX idx_ec_post_del_tg_locale \(deleted_at=\? AND translation_group=\?\)/, + ); + expect(plan).not.toContain("SCAN ec_post"); + expect(plan).not.toContain("idx_ec_post_loc_crt"); + expect(plan).not.toContain("idx_ec_post_loc_upd"); +}); + +it.each([ + { groupCount: 2, publishedOnly: false }, + { groupCount: 2, publishedOnly: true }, + { groupCount: SQL_BATCH_SIZE, publishedOnly: false }, + { groupCount: SQL_BATCH_SIZE, publishedOnly: true }, +])( + "seeks $groupCount batched translation groups through the translation_group index (publishedOnly=$publishedOnly)", + async ({ groupCount, publishedOnly }) => { + const groups = Array.from( + { length: groupCount }, + (_, index) => `tg-${String(index + 1).padStart(4, "0")}`, + ); + + const items = await repo.findTranslationsForGroups("post", groups, { publishedOnly }); + + expect(items).toHaveLength(groupCount * 2); + + const plan = explain(translationQuery()); + expect(contentAccess(plan)).toContain("INDEX idx_ec_post_del_tg_locale"); + expect(plan).not.toContain("SCAN ec_post"); + expect(plan).not.toContain("idx_ec_post_loc_crt"); + expect(plan).not.toContain("idx_ec_post_loc_upd"); + }, +); + +/** + * Menu references resolve a translation group without a `deleted_at` filter, so + * they need an index that leads with `translation_group`. `fr` has no `ec_post` + * row, which exercises the any-locale fallback as well as the direct lookup. + */ +it.each([ + { locale: "en", url: "/post/slug-10" }, + { locale: "fr", url: "/post/slug-9" }, +])("seeks a menu content reference resolved for $locale", async ({ locale, url }) => { + await seedMenuReference("tg-0005"); + + const menu = await getMenuWithDb("primary", db, { locale }); + + expect(menu?.items.map((item) => item.url)).toEqual([url]); + + const queries = translationQueries(); + expect(queries.length).toBeGreaterThan(0); + for (const query of queries) { + const plan = explain(query); + expect(contentAccess(plan)).toContain("INDEX idx_ec_post_tg_locale"); + expect(plan).not.toContain("SCAN ec_post"); + expect(plan).not.toContain("idx_ec_post_loc_crt"); + expect(plan).not.toContain("idx_ec_post_loc_upd"); + } +}); + +it("migrates a pre-055 table off the single-column translation_group index", async () => { + sqlite.exec(`DROP INDEX idx_ec_post_tg_locale`); + sqlite.exec(`DROP INDEX idx_ec_post_del_tg_locale`); + sqlite.exec(`CREATE INDEX idx_ec_post_translation_group ON ec_post (translation_group)`); + captured = []; + await repo.findTranslations("post", "tg-0005"); + expect(contentAccess(explain(translationQuery()))).toContain("idx_ec_post_loc_crt"); + + await migration055.up(db); + + expect(indexNames()).not.toContain("idx_ec_post_translation_group"); + captured = []; + await repo.findTranslations("post", "tg-0005"); + expect(contentAccess(explain(translationQuery()))).toMatch( + /SEARCH ec_post USING (?:COVERING )?INDEX idx_ec_post_del_tg_locale \(deleted_at=\? AND translation_group=\?\)/, + ); + + await seedMenuReference("tg-0005"); + await getMenuWithDb("primary", db, { locale: "en" }); + expect(contentAccess(explain(translationQuery()))).toContain("INDEX idx_ec_post_tg_locale"); +}); + +function indexNames(): string[] { + return ( + sqlite.prepare(`SELECT name FROM sqlite_master WHERE type = 'index'`).all() as { + name: string; + }[] + ).map((row) => row.name); +} + +async function seedMenuReference(referenceGroup: string): Promise { + for (const locale of ["en", "fr"]) { + await db + .insertInto("_emdash_menus") + .values({ id: `menu-${locale}`, name: "primary", label: "Primary", locale }) + .execute(); + await db + .insertInto("_emdash_menu_items") + .values({ + id: `item-${locale}`, + menu_id: `menu-${locale}`, + parent_id: null, + sort_order: 0, + type: "post", + reference_collection: "post", + reference_id: referenceGroup, + custom_url: null, + label: "Post", + title_attr: null, + target: null, + css_classes: null, + locale, + translation_group: null, + }) + .execute(); + } + captured = []; +} + +function translationQueries(): CapturedQuery[] { + return captured.filter((query) => query.sql.includes("translation_group")); +} + +function translationQuery(): CapturedQuery { + const queries = translationQueries(); + expect(queries).toHaveLength(1); + return queries[0]!; +} + +/** better-sqlite3 only binds primitives; coerce values captured from Kysely. */ +function bindable(parameter: unknown): unknown { + if (typeof parameter === "boolean") return parameter ? 1 : 0; + if (parameter instanceof Date) return parameter.toISOString(); + if (parameter === undefined) return null; + return parameter; +} + +function explain(query: CapturedQuery): string { + const rows = sqlite + .prepare(`EXPLAIN QUERY PLAN ${query.sql}`) + .all(...query.parameters.map(bindable)) as { detail: string }[]; + return rows.map((row) => row.detail).join("\n"); +} + +function contentAccess(plan: string): string | undefined { + return plan.split("\n").find((detail) => /\b(?:SCAN|SEARCH) ec_post\b/.test(detail)); +} diff --git a/packages/core/tests/integration/database/migrations.test.ts b/packages/core/tests/integration/database/migrations.test.ts index ce2e6de4c3..df9af0f17a 100644 --- a/packages/core/tests/integration/database/migrations.test.ts +++ b/packages/core/tests/integration/database/migrations.test.ts @@ -141,6 +141,7 @@ describe("Database Migrations (Integration)", () => { "052_media_usage_read_index", "053_plugin_mcp_tools", "054_media_upload_attempts", + "055_content_translation_group_locale_index", ]; await db.deleteFrom("_emdash_migrations").where("name", "in", trailing).execute(); diff --git a/packages/core/tests/integration/i18n/i18n.test.ts b/packages/core/tests/integration/i18n/i18n.test.ts index 7a11e1298a..de0a5a1255 100644 --- a/packages/core/tests/integration/i18n/i18n.test.ts +++ b/packages/core/tests/integration/i18n/i18n.test.ts @@ -86,7 +86,8 @@ describe("i18n (Integration)", () => { const indexNames = result.rows.map((r) => r.name); expect(indexNames).toContain("idx_ec_post_locale"); - expect(indexNames).toContain("idx_ec_post_translation_group"); + expect(indexNames).toContain("idx_ec_post_tg_locale"); + expect(indexNames).toContain("idx_ec_post_del_tg_locale"); }); });