diff --git a/prisma/migrations/0415_foreign_key_and_prefix_indexes/migration.sql b/prisma/migrations/0415_foreign_key_and_prefix_indexes/migration.sql new file mode 100644 index 000000000..1844f11a7 --- /dev/null +++ b/prisma/migrations/0415_foreign_key_and_prefix_indexes/migration.sql @@ -0,0 +1,101 @@ +-- Index hygiene: a leading index for every foreign key that cascades, and +-- no index that is a prefix of another. +-- +-- 1. Twenty-one foreign keys with ON DELETE CASCADE or SET NULL had no index +-- leading with their column. Deleting the parent row (a medication, a +-- schedule, a practitioner, an account) then scans the whole child table +-- once per deleted parent: `medication_intake_events.medication_id` is the +-- one that hurts, scanned in full for every medication an account delete +-- removes. Each gets a plain btree on the column. +-- +-- 2. Seventeen indexes repeated the leading columns of another index on the +-- same table, in the same order and direction, mostly a `(user_id)` index +-- beside a `(user_id, …)` unique key. The longer index answers every +-- lookup the shorter one did, so the shorter one only cost writes and +-- space. They go. `measurements` has its own migration (0416), because a +-- lock on that table weighs differently. +-- +-- `tests/integration/index-hygiene.test.ts` holds the schema to both rules +-- from the catalog. +-- +-- Lock wait. `CREATE INDEX` takes SHARE on its table (reads go on, writes +-- wait while it builds; every table here is small, so the build is quick) +-- and `DROP INDEX` takes ACCESS EXCLUSIVE for the instant the catalog +-- changes. `CREATE INDEX CONCURRENTLY` cannot share a migration file with +-- any other statement (0416), so the whole set runs in one `DO` block: one +-- statement that commits or rolls back whole, with the lock wait bounded at +-- five seconds so a long reader cannot queue the tables' writes behind the +-- migration. Every statement is `IF [NOT] EXISTS`, so a lock timeout leaves +-- the file safe to apply again, which the marker declares +-- (docs/ops/deploy.md, "A migration that timed out on a lock"). +-- +-- @retry-on-lock-timeout + +DO $migration$ +BEGIN + PERFORM set_config('lock_timeout', '5s', true); + + -- 1. Foreign keys without a leading index. + CREATE INDEX IF NOT EXISTS "passkeys_user_id_idx" + ON "passkeys" ("user_id"); + CREATE INDEX IF NOT EXISTS "medication_schedules_medication_id_idx" + ON "medication_schedules" ("medication_id"); + CREATE INDEX IF NOT EXISTS "medication_intake_events_medication_id_idx" + ON "medication_intake_events" ("medication_id"); + CREATE INDEX IF NOT EXISTS "telegram_reminder_messages_schedule_id_idx" + ON "telegram_reminder_messages" ("schedule_id"); + CREATE INDEX IF NOT EXISTS "medication_inventory_items_medication_id_idx" + ON "medication_inventory_items" ("medication_id"); + CREATE INDEX IF NOT EXISTS "medication_side_effects_medication_id_idx" + ON "medication_side_effects" ("medication_id"); + CREATE INDEX IF NOT EXISTS "withings_oauth_states_user_id_idx" + ON "withings_oauth_states" ("user_id"); + CREATE INDEX IF NOT EXISTS "whoop_oauth_states_user_id_idx" + ON "whoop_oauth_states" ("user_id"); + CREATE INDEX IF NOT EXISTS "fitbit_oauth_states_user_id_idx" + ON "fitbit_oauth_states" ("user_id"); + CREATE INDEX IF NOT EXISTS "google_health_oauth_states_user_id_idx" + ON "google_health_oauth_states" ("user_id"); + CREATE INDEX IF NOT EXISTS "whoop_connect_tickets_user_id_idx" + ON "whoop_connect_tickets" ("user_id"); + CREATE INDEX IF NOT EXISTS "cycle_day_logs_cycle_id_idx" + ON "cycle_day_logs" ("cycle_id"); + CREATE INDEX IF NOT EXISTS "invite_tokens_created_by_idx" + ON "invite_tokens" ("created_by"); + CREATE INDEX IF NOT EXISTS "invite_tokens_used_by_idx" + ON "invite_tokens" ("used_by"); + CREATE INDEX IF NOT EXISTS "invite_redemptions_user_id_idx" + ON "invite_redemptions" ("user_id"); + CREATE INDEX IF NOT EXISTS "custom_metric_entries_custom_metric_id_idx" + ON "custom_metric_entries" ("custom_metric_id"); + CREATE INDEX IF NOT EXISTS "encounters_reminder_id_idx" + ON "encounters" ("reminder_id"); + CREATE INDEX IF NOT EXISTS "vaccination_records_encounter_id_idx" + ON "vaccination_records" ("encounter_id"); + CREATE INDEX IF NOT EXISTS "vaccination_records_practitioner_id_idx" + ON "vaccination_records" ("practitioner_id"); + CREATE INDEX IF NOT EXISTS "vaccination_records_reminder_id_idx" + ON "vaccination_records" ("reminder_id"); + CREATE INDEX IF NOT EXISTS "managed_profile_handovers_created_by_id_idx" + ON "managed_profile_handovers" ("created_by_id"); + + -- 2. Indexes that are a prefix of another index on the same table. + DROP INDEX IF EXISTS "biomarkers_user_id_idx"; + DROP INDEX IF EXISTS "clinician_share_link_documents_share_link_id_idx"; + DROP INDEX IF EXISTS "coach_usage_user_id_date_key_idx"; + DROP INDEX IF EXISTS "custom_metrics_user_id_idx"; + DROP INDEX IF EXISTS "cycle_day_logs_user_id_date_idx"; + DROP INDEX IF EXISTS "data_backups_user_id_idx"; + DROP INDEX IF EXISTS "environment_contexts_user_id_date_idx"; + DROP INDEX IF EXISTS "illness_day_logs_episode_id_idx"; + DROP INDEX IF EXISTS "integration_statuses_user_id_idx"; + DROP INDEX IF EXISTS "medications_user_id_idx"; + DROP INDEX IF EXISTS "menstrual_cycles_user_id_start_date_idx"; + DROP INDEX IF EXISTS "mood_entries_user_id_date_idx"; + DROP INDEX IF EXISTS "notification_channels_user_id_idx"; + DROP INDEX IF EXISTS "push_subscriptions_user_id_idx"; + DROP INDEX IF EXISTS "refresh_tokens_user_id_idx"; + DROP INDEX IF EXISTS "user_achievements_user_id_idx"; + DROP INDEX IF EXISTS "user_known_devices_user_id_idx"; +END +$migration$; diff --git a/prisma/migrations/0416_drop_measurements_type_prefix_index/migration.sql b/prisma/migrations/0416_drop_measurements_type_prefix_index/migration.sql new file mode 100644 index 000000000..ff86f6c2c --- /dev/null +++ b/prisma/migrations/0416_drop_measurements_type_prefix_index/migration.sql @@ -0,0 +1,17 @@ +-- Drop `measurements_user_id_type_measured_at_idx`. +-- +-- `(user_id, type, measured_at)` is the leading part of the unique key +-- `(user_id, type, measured_at, source, sleep_stage)`, and live reads have +-- `measurements_live_covering_idx` besides. Every query shape that used the +-- shorter index plans the same way on the unique key, at a cost within a few +-- percent (`tests/integration/index-hygiene.test.ts` compares the plans on a +-- seeded table). The index cost every write to the densest table in the +-- schema and, on a bloated instance, more space than the table itself. +-- +-- `DROP INDEX CONCURRENTLY` takes no lock that blocks reads or writes: it +-- waits for the transactions already using the index to finish and then +-- removes it. It refuses to run inside a transaction block, and under +-- `migrate deploy` it does so whenever the file holds any other statement +-- (Prisma 7 on Postgres 16 reports 25001), so this file holds exactly this +-- one statement. `IF EXISTS` makes a re-run a no-op. +DROP INDEX CONCURRENTLY IF EXISTS "measurements_user_id_type_measured_at_idx"; diff --git a/prisma/schema.prisma b/prisma/schema.prisma index e1975b2fc..63fa6d26a 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -1248,7 +1248,6 @@ model EnvironmentContext { updatedAt DateTime @updatedAt @map("updated_at") @@unique([userId, date]) - @@index([userId, date]) // `environment_contexts_aq_gap_idx` (user_id, date) WHERE aq_fetched_at IS // NULL is a partial index from migration 0381 (raw SQL only). @@map("environment_contexts") @@ -1300,6 +1299,8 @@ model Passkey { user User @relation(fields: [userId], references: [id], onDelete: Cascade) + // Leads the foreign key, so a cascade from the parent row is an index lookup. + @@index([userId]) @@map("passkeys") } @@ -1484,7 +1485,6 @@ model UserKnownDevice { // twice for one account, so the "first sighting → alert" decision is a // single unique-constrained upsert. @@unique([userId, deviceHash]) - @@index([userId]) @@map("user_known_devices") } @@ -2088,7 +2088,6 @@ model Measurement { // v1.4.23 — Apple Health batch dedup key. NULL externalId is distinct // (Postgres semantics) so manual entries don't collide here. @@unique([userId, type, source, externalId]) - @@index([userId, type, measuredAt]) // v1.25 — type-less `(userId, measuredAt)` range scan. The bulk feature read // (`extractFeatures`) and consolidation scans walk a user's measurements by // time with no `type` predicate; `(userId, type, measuredAt)` can't serve @@ -3003,7 +3002,6 @@ model Medication { /// re-mirror of the same concept id returns the existing row (the /// create route pre-queries the triple and catches the P2002 race). @@unique([userId, externalSource, externalId]) - @@index([userId]) @@index([treatmentClass]) @@map("medications") } @@ -3213,6 +3211,8 @@ model MedicationSchedule { medication Medication @relation(fields: [medicationId], references: [id], onDelete: Cascade) telegramMessages TelegramReminderMessage[] + // Leads the foreign key, so a cascade from the parent row is an index lookup. + @@index([medicationId]) @@map("medication_schedules") } @@ -3491,6 +3491,8 @@ model MedicationIntakeEvent { // without this one the purge planned a sequential scan and could not finish // inside statement_timeout. Raw SQL only (partial predicate not expressible // in Prisma). + // Leads the foreign key, so a cascade from the parent row is an index lookup. + @@index([medicationId]) @@map("medication_intake_events") } @@ -3655,6 +3657,8 @@ model MedicationInventoryItem { // `(user_id) WHERE notes IS NOT NULL AND notes_encrypted IS NULL` — see // migration 0243 (v1.28.46 H4) for the med-notes boot-discovery scan. Raw // SQL only (partial predicate not expressible in Prisma). + // Leads the foreign key, so a cascade from the parent row is an index lookup. + @@index([medicationId]) @@map("medication_inventory_items") } @@ -3718,6 +3722,8 @@ model MedicationSideEffect { // `(user_id) WHERE notes IS NOT NULL AND notes_encrypted IS NULL` — see // migration 0243 (v1.28.46 H4) for the med-notes boot-discovery scan. Raw // SQL only (partial predicate not expressible in Prisma). + // Leads the foreign key, so a cascade from the parent row is an index lookup. + @@index([medicationId]) @@map("medication_side_effects") } @@ -3809,6 +3815,8 @@ model TelegramReminderMessage { @@unique([recipientUserId, medicationId, scheduleId, date, phase, timeOfDay], map: "telegram_reminder_messages_recipient_slot_key") @@index([medicationId, date]) + // Leads the foreign key, so a cascade from the parent row is an index lookup. + @@index([scheduleId]) @@map("telegram_reminder_messages") } @@ -4004,6 +4012,9 @@ model InviteToken { redemptions InviteRedemption[] @@index([expiresAt]) + // Leads the foreign key, so a cascade from the parent row is an index lookup. + @@index([createdBy]) + @@index([usedBy]) @@map("invite_tokens") } @@ -4047,6 +4058,8 @@ model ManagedProfileHandover { @@index([profileId]) // `managed_profile_handovers_open_idx` UNIQUE (profile_id) WHERE used_at IS // NULL AND revoked_at IS NULL is a partial index from migration 0383. + // Leads the foreign key, so a cascade from the parent row is an index lookup. + @@index([createdById]) @@map("managed_profile_handovers") } @@ -4100,6 +4113,8 @@ model InviteRedemption { user User? @relation("InviteRedemptions", fields: [userId], references: [id], onDelete: SetNull) @@index([inviteId]) + // Leads the foreign key, so a cascade from the parent row is an index lookup. + @@index([userId]) @@map("invite_redemptions") } @@ -4136,7 +4151,6 @@ model RefreshToken { user User @relation(fields: [userId], references: [id], onDelete: Cascade) - @@index([userId]) /// Composite index — every reuse-detection / device-scoped lookup filters /// on (userId, deviceId), so a single-column device_id index would never /// be used while costing writes. Multiple live rows per (userId, deviceId) @@ -4267,6 +4281,8 @@ model WithingsOAuthState { user User @relation(fields: [userId], references: [id], onDelete: Cascade) @@index([expiresAt]) + // Leads the foreign key, so a cascade from the parent row is an index lookup. + @@index([userId]) @@map("withings_oauth_states") } @@ -4348,6 +4364,8 @@ model WhoopOAuthState { user User @relation(fields: [userId], references: [id], onDelete: Cascade) @@index([expiresAt]) + // Leads the foreign key, so a cascade from the parent row is an index lookup. + @@index([userId]) @@map("whoop_oauth_states") } @@ -4380,6 +4398,8 @@ model WhoopConnectTicket { user User @relation(fields: [userId], references: [id], onDelete: Cascade) @@index([expiresAt]) + // Leads the foreign key, so a cascade from the parent row is an index lookup. + @@index([userId]) @@map("whoop_connect_tickets") } @@ -4434,6 +4454,8 @@ model FitbitOAuthState { user User @relation(fields: [userId], references: [id], onDelete: Cascade) @@index([expiresAt]) + // Leads the foreign key, so a cascade from the parent row is an index lookup. + @@index([userId]) @@map("fitbit_oauth_states") } @@ -4512,6 +4534,8 @@ model GoogleHealthOAuthState { user User @relation(fields: [userId], references: [id], onDelete: Cascade) @@index([expiresAt]) + // Leads the foreign key, so a cascade from the parent row is an index lookup. + @@index([userId]) @@map("google_health_oauth_states") } @@ -4642,7 +4666,6 @@ model MoodEntry { // that carry a source-stable id upsert on this; native/MANUAL entries // (NULL externalId) are distinct here and stay on the legacy key above. @@unique([userId, source, externalId]) - @@index([userId, date]) // v1.12.1 (P1-3) — mood-insights reads filter `{ userId, deletedAt: // null, moodLoggedAt: { gte } }` ordered by `moodLoggedAt desc`; this // index turns that into a range scan. See migration 0123. @@ -5098,7 +5121,6 @@ model IntegrationStatus { user User @relation(fields: [userId], references: [id], onDelete: Cascade) @@unique([userId, integration]) - @@index([userId]) @@index([state]) @@map("integration_statuses") } @@ -5451,7 +5473,6 @@ model NotificationChannel { preferences NotificationPreference[] @@unique([userId, type]) - @@index([userId]) @@map("notification_channels") } @@ -5479,7 +5500,6 @@ model PushSubscription { user User @relation(fields: [userId], references: [id], onDelete: Cascade) @@unique([userId, endpoint]) - @@index([userId]) @@map("push_subscriptions") } @@ -5526,7 +5546,6 @@ model DataBackup { chunks DataBackupChunk[] @@unique([userId, type], map: "data_backups_user_id_type_key") - @@index([userId]) @@map("data_backups") } @@ -6369,7 +6388,6 @@ model Biomarker { mergesInto BiomarkerMerge[] @@unique([userId, name]) - @@index([userId]) @@map("biomarkers") } @@ -6412,7 +6430,6 @@ model CustomMetric { entries CustomMetricEntry[] @@unique([userId, name]) - @@index([userId]) @@map("custom_metrics") } @@ -6450,6 +6467,8 @@ model CustomMetricEntry { // `(user_id) WHERE note IS NOT NULL` — see // migration 0357 for the free-text encryption boot-discovery scan. Raw SQL // only (partial predicate not expressible in Prisma). + // Leads the foreign key, so a cascade from the parent row is an index lookup. + @@index([customMetricId]) @@map("custom_metric_entries") } @@ -6497,7 +6516,6 @@ model UserAchievement { user User @relation(fields: [userId], references: [id], onDelete: Cascade) @@unique([userId, achievementId]) - @@index([userId]) @@map("user_achievements") } @@ -7189,7 +7207,6 @@ model CoachUsage { user User @relation(fields: [userId], references: [id], onDelete: Cascade) @@unique([userId, dateKey]) - @@index([userId, dateKey]) @@map("coach_usage") } @@ -7440,7 +7457,6 @@ model ClinicianShareLinkDocument { document InboundDocument @relation(fields: [documentId], references: [id], onDelete: Cascade) @@unique([shareLinkId, documentId]) - @@index([shareLinkId]) @@index([documentId]) @@map("clinician_share_link_documents") } @@ -8031,7 +8047,6 @@ model MenstrualCycle { dayLogs CycleDayLog[] @@unique([userId, startDate]) - @@index([userId, startDate]) // Keyset support for the `/api/sync/changes` cycle feed // (`where userId` ordered by `(updatedAt asc, id asc)`). @@index([userId, updatedAt, id]) @@ -8131,9 +8146,10 @@ model CycleDayLog { // NULL-distinct external dedup key for HealthKit re-ingest (the // MoodEntry dual-unique precedent). @@unique([userId, source, externalId]) - @@index([userId, date]) // Keyset support for the `/api/sync/changes` cycle-day feed. @@index([userId, updatedAt, id]) + // Leads the foreign key, so a cascade from the parent row is an index lookup. + @@index([cycleId]) @@map("cycle_day_logs") } @@ -8325,7 +8341,6 @@ model IllnessDayLog { @@unique([episodeId, date]) @@index([userId, date]) - @@index([episodeId]) @@map("illness_day_logs") } @@ -9332,6 +9347,8 @@ model Encounter { @@index([userId, deletedAt, occurredAt]) @@index([userId, status, occurredAt]) @@index([practitionerId]) + // Leads the foreign key, so a cascade from the parent row is an index lookup. + @@index([reminderId]) @@map("encounters") } @@ -9592,6 +9609,10 @@ model VaccinationRecord { @@index([userId, deletedAt, occurredAt]) @@index([userId, antigenSlug, occurredAt]) @@index([customVaccineId]) + // Leads the foreign key, so a cascade from the parent row is an index lookup. + @@index([encounterId]) + @@index([practitionerId]) + @@index([reminderId]) @@map("vaccination_records") } diff --git a/src/__tests__/migration-lock-timeout-guard.test.ts b/src/__tests__/migration-lock-timeout-guard.test.ts index 6afe05a6e..45c7595bf 100644 --- a/src/__tests__/migration-lock-timeout-guard.test.ts +++ b/src/__tests__/migration-lock-timeout-guard.test.ts @@ -89,6 +89,56 @@ function violations(name: string, sql: string): string[] { return found; } +/** + * `CREATE INDEX CONCURRENTLY` / `DROP INDEX CONCURRENTLY` refuse to run in a + * transaction block, and under `migrate deploy` they do so whenever the file + * holds any other statement (SQLSTATE 25001, seen against Prisma 7 and + * Postgres 16). Such a statement therefore stands alone in its file. + */ +function concurrentViolations(name: string, sql: string): string[] { + const statements = topLevelStatements(sql); + const concurrent = statements.filter((statement) => + /\bCONCURRENTLY\b/i.test(statement), + ); + if (concurrent.length === 0 || statements.length === 1) return []; + return [ + `${name}: a CONCURRENTLY statement must be the only statement in its file`, + ]; +} + +describe("concurrent index statements", () => { + it("stand alone in their migration file", () => { + const found = migrations().flatMap(({ name, sql }) => + concurrentViolations(name, sql), + ); + expect(found, found.join("\n")).toEqual([]); + }); + + it("sees the concurrent migrations there are", () => { + const concurrent = migrations() + .filter(({ sql }) => + /\bCONCURRENTLY\b/i.test(topLevelStatements(sql).join(";")), + ) + .map(({ name }) => name); + expect(concurrent).toContain("0416_drop_measurements_type_prefix_index"); + }); + + it("refuses a concurrent statement beside another one", () => { + expect( + concurrentViolations( + "a", + 'DROP INDEX CONCURRENTLY IF EXISTS "x";\nDROP INDEX CONCURRENTLY IF EXISTS "y";', + ), + ).toHaveLength(1); + expect( + concurrentViolations( + "b", + '-- CONCURRENTLY in a comment\nDROP INDEX CONCURRENTLY IF EXISTS "x";', + ), + ).toEqual([]); + }); +}); + describe("migration lock waits", () => { it("bounds lock waits where they take effect and marks only repeatable files", () => { const found = migrations().flatMap(({ name, sql }) => @@ -97,11 +147,13 @@ describe("migration lock waits", () => { expect(found, found.join("\n")).toEqual([]); }); - it("sees the one migration that bounds its wait today", () => { + it("sees the migrations that bound their wait", () => { const marked = migrations() .filter(({ sql }) => RETRY_MARKER.test(sql)) .map(({ name }) => name); expect(marked).toContain("0412_measurement_hot_updates"); + expect(marked).toContain("0414_drop_medication_compliance_rollups"); + expect(marked).toContain("0415_foreign_key_and_prefix_indexes"); }); it("catches each shape it exists to refuse", () => { diff --git a/src/lib/personal-records/pr-detection-worker.ts b/src/lib/personal-records/pr-detection-worker.ts index a6e468b77..e2839662a 100644 --- a/src/lib/personal-records/pr-detection-worker.ts +++ b/src/lib/personal-records/pr-detection-worker.ts @@ -19,8 +19,8 @@ * - All-time scan rather than a rolling window. The * `personal_records_user_metric_idx` index makes * `ORDER BY value LIMIT 1` constant-time; reading the worst-case - * ~30k measurement rows per metric is ~50 ms with the typed - * `measurements_user_id_type_measured_at_idx` index. + * ~30k measurement rows per metric is ~50 ms over the + * `(user_id, type, measured_at, …)` unique index. * - Tie handling: an equal-value reading writes a row but the caller * suppresses the push notification (the user matched, not beat * their best). The notification suppression lives on the job diff --git a/tests/integration/index-hygiene.test.ts b/tests/integration/index-hygiene.test.ts new file mode 100644 index 000000000..370eb8df5 --- /dev/null +++ b/tests/integration/index-hygiene.test.ts @@ -0,0 +1,360 @@ +/** + * Index hygiene, read from the catalog of the migrated database. + * + * Two rules, each with its counter-check: + * + * 1. Every foreign key whose delete action does work in the child table + * (CASCADE, SET NULL, SET DEFAULT) has an index leading with its columns. + * Without one, deleting a parent row scans the child table once per + * deleted parent; an account delete runs hundreds of them. + * 2. No btree index is a prefix of another btree index on the same table + * (same leading columns, operator classes, collations and directions, + * same predicate). The longer index answers every lookup the shorter one + * did, so the shorter one only costs writes and space. A unique index is + * never counted as the redundant one: it enforces something. + * + * And the evidence for migration 0416: on a seeded `measurements` table of + * production size, every query shape that used + * `measurements_user_id_type_measured_at_idx` plans the same way without it, + * on an index, at a cost within a few percent. + */ +import { Client } from "pg"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; + +import { getPrismaClient, truncateAllTables } from "./setup"; + +/** + * Foreign keys allowed to go without a leading index, with the reason. + * Empty: every one has its index. + */ +const FK_EXCEPTIONS: Record = {}; + +/** Prefix indexes allowed to stay, with the reason. Empty. */ +const PREFIX_EXCEPTIONS: Record = {}; + +async function connect(): Promise { + const raw = process.env.DATABASE_URL; + if (!raw) throw new Error("DATABASE_URL is not set for the integration run"); + const url = new URL(raw); + url.searchParams.delete("schema"); + url.searchParams.delete("pgbouncer"); + const client = new Client({ connectionString: url.toString() }); + await client.connect(); + return client; +} + +const FK_WITHOUT_LEADING_INDEX = ` + SELECT c.conrelid::regclass::text AS tbl, c.conname AS name + FROM pg_constraint c + WHERE c.contype = 'f' + AND c.connamespace = 'public'::regnamespace + AND c.confdeltype IN ('c', 'n', 'd') + AND NOT EXISTS ( + SELECT 1 FROM pg_index i + WHERE i.indrelid = c.conrelid + AND i.indpred IS NULL + AND i.indexprs IS NULL + AND (string_to_array(i.indkey::text, ' ')::int2[])[1:array_length(c.conkey, 1)] @> c.conkey + AND (string_to_array(i.indkey::text, ' ')::int2[])[1:array_length(c.conkey, 1)] <@ c.conkey + ) + ORDER BY 1, 2`; + +const ACTING_FK_COUNT = ` + SELECT count(*)::int AS n FROM pg_constraint + WHERE contype = 'f' AND connamespace = 'public'::regnamespace + AND confdeltype IN ('c', 'n', 'd')`; + +/* + * `indkey`, `indclass`, `indcollation` and `indoption` are vectors whose array + * casts start at index 0; going through text gives ordinary 1-based arrays. + */ +const PREFIX_INDEXES = ` + WITH idx AS ( + SELECT i.indexrelid, i.indrelid, c.relname AS index_name, + t.relname AS table_name, i.indisunique, i.indisprimary, + string_to_array(i.indkey::text, ' ')::int[] AS cols, + string_to_array(i.indclass::text, ' ')::oid[] AS opc, + string_to_array(i.indcollation::text, ' ')::oid[] AS coll, + string_to_array(i.indoption::text, ' ')::int[] AS opt, + pg_get_expr(i.indpred, i.indrelid) AS pred, + i.indexprs IS NOT NULL AS has_exprs, am.amname + FROM pg_index i + JOIN pg_class c ON c.oid = i.indexrelid + JOIN pg_class t ON t.oid = i.indrelid + JOIN pg_am am ON am.oid = c.relam + WHERE t.relnamespace = 'public'::regnamespace + ) + SELECT a.table_name AS tbl, a.index_name AS redundant, b.index_name AS covered_by + FROM idx a + JOIN idx b ON a.indrelid = b.indrelid AND a.indexrelid <> b.indexrelid + WHERE a.amname = 'btree' AND b.amname = 'btree' + AND NOT a.indisprimary AND NOT a.has_exprs AND NOT b.has_exprs + AND a.pred IS NOT DISTINCT FROM b.pred + AND cardinality(a.cols) <= cardinality(b.cols) + AND a.cols = b.cols[1:cardinality(a.cols)] + AND a.opc = b.opc[1:cardinality(a.cols)] + AND a.coll = b.coll[1:cardinality(a.cols)] + AND a.opt = b.opt[1:cardinality(a.cols)] + AND (NOT a.indisunique OR (b.indisunique AND a.cols = b.cols)) + AND (cardinality(a.cols) < cardinality(b.cols) + OR (NOT a.indisunique AND b.indisunique) + OR (a.indisunique = b.indisunique AND a.indexrelid > b.indexrelid)) + ORDER BY 1, 2`; + +const BTREE_INDEX_COUNT = ` + SELECT count(*)::int AS n FROM pg_index i + JOIN pg_class c ON c.oid = i.indexrelid JOIN pg_am am ON am.oid = c.relam + WHERE c.relnamespace = 'public'::regnamespace AND am.amname = 'btree'`; + +describe("index hygiene", () => { + let db: Client; + beforeAll(async () => { + db = await connect(); + }); + afterAll(async () => { + await db.end(); + }); + + it("gives every acting foreign key a leading index", async () => { + const examined = (await db.query(ACTING_FK_COUNT)).rows[0].n as number; + // The matcher has to have something to match: the schema has well over a + // hundred cascading foreign keys. + expect(examined).toBeGreaterThan(100); + const { rows } = await db.query(FK_WITHOUT_LEADING_INDEX); + const offenders = rows + .map((r) => `${r.tbl}.${r.name}`) + .filter((key) => !(key in FK_EXCEPTIONS)); + expect(offenders, offenders.join("\n")).toEqual([]); + }); + + it("counter-check: flags a cascading foreign key without an index", async () => { + await db.query("BEGIN"); + try { + await db.query( + `CREATE TABLE "zz_fk_probe" ("id" text PRIMARY KEY, + "user_id" text REFERENCES "users"("id") ON DELETE CASCADE)`, + ); + const flagged = (await db.query(FK_WITHOUT_LEADING_INDEX)).rows.map( + (r) => r.tbl, + ); + expect(flagged).toContain("zz_fk_probe"); + await db.query(`CREATE INDEX ON "zz_fk_probe" ("user_id", "id")`); + const after = (await db.query(FK_WITHOUT_LEADING_INDEX)).rows.map( + (r) => r.tbl, + ); + expect(after).not.toContain("zz_fk_probe"); + } finally { + await db.query("ROLLBACK"); + } + }); + + it("keeps no btree index that is a prefix of another", async () => { + const examined = (await db.query(BTREE_INDEX_COUNT)).rows[0].n as number; + expect(examined).toBeGreaterThan(300); + const { rows } = await db.query(PREFIX_INDEXES); + const offenders = rows + .filter((r) => !(r.redundant in PREFIX_EXCEPTIONS)) + .map((r) => `${r.tbl}: ${r.redundant} is a prefix of ${r.covered_by}`); + expect(offenders, offenders.join("\n")).toEqual([]); + }); + + it("counter-check: flags a prefix and a duplicate, but not a reversed direction", async () => { + await db.query("BEGIN"); + try { + await db.query( + `CREATE INDEX "zz_prefix_probe" ON "measurements" ("user_id", "type")`, + ); + await db.query( + `CREATE INDEX "zz_duplicate_probe" ON "users" ("username")`, + ); + await db.query( + `CREATE INDEX "zz_direction_probe" ON "measurements" ("user_id", "type", "measured_at" DESC, "source")`, + ); + const flagged = (await db.query(PREFIX_INDEXES)).rows.map( + (r) => r.redundant, + ); + expect(flagged).toContain("zz_prefix_probe"); + expect(flagged).toContain("zz_duplicate_probe"); + // `users_username_key` is unique: it enforces something, so it is + // never the one reported, even beside a plain twin. + expect(flagged).not.toContain("users_username_key"); + // The live-covering index runs `measured_at DESC` too, but carries a + // predicate, so it is not a prefix of the probe either way. + expect(flagged).not.toContain("zz_direction_probe"); + } finally { + await db.query("ROLLBACK"); + } + }); +}); + +describe("measurements without (user_id, type, measured_at)", () => { + const DROPPED = "measurements_user_id_type_measured_at_idx"; + const USERS = 5; + const ROWS = 130_000; + let db: Client; + let userId: string; + + beforeAll(async () => { + const prisma = getPrismaClient(); + await truncateAllTables(prisma); + const ids: string[] = []; + for (let i = 0; i < USERS; i++) { + const user = await prisma.user.create({ + data: { + username: `plan-probe-${i}`, + email: `plan-probe-${i}@example.test`, + role: "USER", + }, + }); + ids.push(user.id); + } + userId = ids[2]; + db = await connect(); + // Production's mix: dense pulse and steps, a long tail of manual types, + // three sources, one row in twenty a tombstone, a year of history. + await db.query( + `WITH t(type, unit, weight) AS (VALUES + ('PULSE'::measurement_type, 'bpm', 50), ('ACTIVITY_STEPS', 'count', 10), + ('HEART_RATE_VARIABILITY', 'ms', 10), ('SLEEP_DURATION', 'h', 8), + ('BLOOD_GLUCOSE', 'mg/dL', 5), ('OXYGEN_SATURATION', '%', 4), + ('WEIGHT', 'kg', 3), ('BLOOD_PRESSURE_SYS', 'mmHg', 3), + ('BLOOD_PRESSURE_DIA', 'mmHg', 3), ('RESTING_HEART_RATE', 'bpm', 2), + ('BODY_FAT', '%', 2)), + slots AS (SELECT row_number() OVER () AS n, type, unit + FROM t, generate_series(1, t.weight)) + INSERT INTO "measurements" + ("id", "user_id", "type", "value", "unit", "source", "measured_at", + "updated_at", "deleted_at") + SELECT 'plan-' || g, ($1::text[])[1 + g % $2], s.type, random() * 100, s.unit, + (ARRAY['APPLE_HEALTH', 'MANUAL', 'WITHINGS']::measurement_source[])[1 + g % 3], + timestamp '2026-10-20' - (g * interval '241 seconds'), + now(), CASE WHEN g % 20 = 0 THEN now() END + FROM generate_series(1, $3) g + JOIN slots s ON s.n = 1 + (g * 7 % 100)`, + [ids, USERS, ROWS], + ); + await db.query(`ANALYZE "measurements"`); + }, 120_000); + + afterAll(async () => { + await db.end(); + await truncateAllTables(getPrismaClient()); + }); + + /** The query shapes the read and write paths send with a user and a type. */ + const QUERIES: Array<[string, string]> = [ + [ + "live range, newest first", + `SELECT * FROM measurements WHERE user_id = $1 AND type = 'WEIGHT' + AND measured_at >= '2026-07-20' AND deleted_at IS NULL + ORDER BY measured_at DESC`, + ], + [ + "range including tombstones", + `SELECT * FROM measurements WHERE user_id = $1 AND type = 'WEIGHT' + AND measured_at >= '2026-07-20' ORDER BY measured_at DESC`, + ], + [ + "latest of a type", + `SELECT max(measured_at) FROM measurements WHERE user_id = $1 AND type = 'PULSE'`, + ], + [ + "count of a type", + `SELECT count(*) FROM measurements WHERE user_id = $1 AND type = 'BLOOD_GLUCOSE'`, + ], + [ + "point lookup without source", + `SELECT id FROM measurements WHERE user_id = $1 AND type = 'PULSE' + AND measured_at = '2026-10-01 10:00'`, + ], + [ + "day aggregate", + `SELECT date_trunc('day', measured_at), avg(value) FROM measurements + WHERE user_id = $1 AND type = 'PULSE' AND measured_at >= '2026-10-01' + AND measured_at < '2026-10-08' AND deleted_at IS NULL GROUP BY 1`, + ], + [ + "two types over a month", + `SELECT * FROM measurements WHERE user_id = $1 + AND type IN ('BLOOD_PRESSURE_SYS', 'BLOOD_PRESSURE_DIA') + AND measured_at BETWEEN '2026-09-01' AND '2026-10-01'`, + ], + [ + "last before a date", + `SELECT * FROM measurements WHERE user_id = $1 AND type = 'SLEEP_DURATION' + AND measured_at < '2026-10-01' ORDER BY measured_at DESC LIMIT 1`, + ], + [ + "tombstone a day", + `UPDATE measurements SET deleted_at = now() WHERE user_id = $1 + AND type = 'HEART_RATE_VARIABILITY' AND measured_at >= '2026-10-10' + AND measured_at < '2026-10-11'`, + ], + ]; + + interface PlanNode { + "Node Type": string; + "Relation Name"?: string; + "Index Name"?: string; + "Total Cost": number; + Plans?: PlanNode[]; + } + + function walk(node: PlanNode, out: PlanNode[] = []): PlanNode[] { + out.push(node); + for (const child of node.Plans ?? []) walk(child, out); + return out; + } + + async function plans(): Promise> { + const result = []; + for (const [, sql] of QUERIES) { + const { rows } = await db.query(`EXPLAIN (FORMAT JSON) ${sql}`, [userId]); + const root = rows[0]["QUERY PLAN"][0].Plan as PlanNode; + result.push({ cost: root["Total Cost"], nodes: walk(root) }); + } + return result; + } + + it("is gone after the migrations", async () => { + const { rowCount } = await db.query( + `SELECT 1 FROM pg_indexes WHERE indexname = $1`, + [DROPPED], + ); + expect(rowCount).toBe(0); + }); + + it("plans every user-and-type query on an index, at the cost it had with it", async () => { + // The state before migration 0416, rebuilt inside a transaction. + await db.query("BEGIN"); + let before; + try { + await db.query( + `CREATE INDEX "${DROPPED}" ON "measurements" ("user_id", "type", "measured_at")`, + ); + before = await plans(); + } finally { + await db.query("ROLLBACK"); + } + const after = await plans(); + + // The comparison means something only if the old index was in use. + const usedBefore = before.filter((p) => + p.nodes.some((n) => n["Index Name"] === DROPPED), + ).length; + expect(usedBefore).toBeGreaterThanOrEqual(3); + + QUERIES.forEach(([label], i) => { + const seqScan = after[i].nodes.find( + (n) => + n["Node Type"] === "Seq Scan" && + n["Relation Name"] === "measurements", + ); + expect(seqScan, `${label}: sequential scan`).toBeUndefined(); + expect( + after[i].cost, + `${label}: cost ${after[i].cost} against ${before[i].cost}`, + ).toBeLessThanOrEqual(before[i].cost * 1.05 + 1); + }); + }, 60_000); +});