Fix two confirmed migration bugs found during scalability investigation

apps/api/migrations/007_hot_path_indexes.sql

@@ -1,22 +1,45 @@
 -- Migration 007: Missing indexes on hot-path queries in routes/importers.ts (issue #257)
 -- Up
 --
--- Analysis identified five index gaps causing sequential scans on the most
+-- Analysis identified index gaps causing sequential scans on the most
 -- frequently hit query paths at production data volumes:
 --
 --   Query                              Missing index
 --   ─────────────────────────────────  ──────────────────────────────────────────
 --   importers WHERE user_id = $1       importers.user_id
 --   importers ORDER BY created_at DESC importers.created_at DESC
---   contract_events WHERE importer_id  contract_events(importer_id, created_at, id)
---     + cursor keyset pagination
 --   bonds WHERE importer_id ORDER BY   bonds(importer_id, created_at DESC)
 --     created_at DESC
 --   tariff_uploads WHERE importer_id   tariff_uploads(importer_id, created_at DESC)
 --     ORDER BY created_at DESC LIMIT 1
 --
 -- All indexes use CONCURRENTLY to avoid table-level locks in production.
 -- See docs/query-analysis.md for full EXPLAIN ANALYZE output and cost table.
+--
+-- #1095 correction: this file originally also declared
+--   CREATE INDEX CONCURRENTLY idx_contract_events_importer_created_at
+--     ON contract_events(importer_id, created_at DESC, id DESC);
+-- contract_events has been a partitioned table (PARTITION BY RANGE
+-- (created_at)) since migration 0002_partition_contract_events.ts, and
+-- PostgreSQL does not support CREATE INDEX CONCURRENTLY directly on a
+-- partitioned table — every run of this file failed on that one statement.
+-- Because `psql -f` does not abort on a single statement error, this went
+-- unnoticed: the other four indexes below were created successfully and
+-- the failure was silent unless someone was watching the output.
+--
+-- The intended target query, GET /:id/events (cursor pagination in
+-- routes/importers.ts), orders by `id DESC` alone — it never sorts by
+-- created_at. That's already fully covered by
+-- idx_contract_events_importer_id_pagination(importer_id, id DESC), which
+-- 0002 declared on the contract_events parent (so it auto-propagates to
+-- every partition, current and future). The two remaining contract_events
+-- queries that do filter by created_at (routes/importers.ts GET
+-- /admin/events, routes/regulatory.ts's claims-filed query) have no
+-- importer_id predicate, so they're served by the BRIN index
+-- idx_contract_events_created_at_brin, also from 0002.
+-- So the statement is dropped outright rather than reworked into a
+-- per-partition CONCURRENTLY + ATTACH PARTITION sequence — there is no
+-- query in this codebase it would actually serve.
 
 -- importers.user_id: supports both the "does this user already have an importer?"
 -- existence check (POST /) and the per-user importer list (GET / non-admin path).
@@ -28,14 +51,6 @@ CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_importers_user_id
 CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_importers_created_at
   ON importers(created_at DESC);
 
--- contract_events(importer_id, created_at DESC, id DESC): covers both the initial
--- event-history fetch and the cursor-keyset continuation clause
--- `(created_at, id) < ($2::timestamptz, $3::uuid)` used in GET /:id/events.
--- The compound index allows the planner to satisfy ORDER BY created_at DESC, id DESC
--- via an Index Scan rather than a sequential scan + sort.
-CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_contract_events_importer_created_at
-  ON contract_events(importer_id, created_at DESC, id DESC);
-
 -- bonds(importer_id, created_at DESC): supports GET /:id/bonds which fetches the
 -- full bond history for an importer ordered by created_at DESC.
 CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_bonds_importer_created_at

apps/api/src/migrations/0005_scalability_indexes.ts

@@ -7,6 +7,10 @@
 
 import type { PoolClient } from 'pg';
 
+// CREATE INDEX CONCURRENTLY is rejected by Postgres inside any transaction
+// block — see runner.ts's MigrationModule.nonTransactional doc comment.
+export const nonTransactional = true;
+
 export const up = async (client: PoolClient): Promise<void> => {
   // 1. Composite index for surety license listing pagination
   // Supports: ORDER BY created_at DESC, id DESC with cursor-based pagination

apps/api/src/migrations/runner.ts

@@ -1,11 +1,28 @@
 import fs from 'fs/promises';
 import path from 'path';
 import { fileURLToPath, pathToFileURL } from 'url';
+import type { PoolClient } from 'pg';
 import { pool } from '../db.js';
 
 const __filename = fileURLToPath(import.meta.url);
 const __dirname = path.dirname(__filename);
 
+interface MigrationModule {
+  up: (client: PoolClient) => Promise<void>;
+  down: (client: PoolClient) => Promise<void>;
+  // Set by a migration that runs CREATE/DROP INDEX CONCURRENTLY (or any
+  // other statement Postgres refuses to run inside a transaction block —
+  // see PreventInTransactionBlock in Postgres's own source). Such a
+  // migration is run on its own connection with no surrounding BEGIN/COMMIT,
+  // so it does not get the same all-or-nothing rollback guarantee the rest
+  // of the chain gets: if it fails partway through, whatever it already
+  // created stays behind and must be cleaned up manually before retrying.
+  // CONCURRENTLY is designed to leave an INVALID index behind on failure
+  // rather than silently rolling back, precisely so it never takes a table
+  // lock — so this is Postgres's own tradeoff, not one this runner adds.
+  nonTransactional?: boolean;
+}
+
 export async function runMigrations(action: 'up' | 'rollback' = 'up'): Promise<void> {
   const client = await pool.connect();
   try {
@@ -55,27 +72,48 @@ export async function runMigrations(action: 'up' | 'rollback' = 'up'): Promise<v
       }
 
       console.log(`Running ${pending.length} pending migrations...`);
-      await client.query('BEGIN');
-      try {
-        for (const m of pending) {
-          const filePath = path.join(__dirname, m.filename);
-          const fileUrl = pathToFileURL(filePath).href;
-          const mod = await import(fileUrl);
-          if (typeof mod.up !== 'function') {
-            throw new Error(`Migration ${m.filename} does not export an up function.`);
+      for (const m of pending) {
+        const filePath = path.join(__dirname, m.filename);
+        const fileUrl = pathToFileURL(filePath).href;
+        const mod = (await import(fileUrl)) as MigrationModule;
+        if (typeof mod.up !== 'function') {
+          throw new Error(`Migration ${m.filename} does not export an up function.`);
+        }
+
+        if (mod.nonTransactional) {
+          // CREATE/DROP INDEX CONCURRENTLY (and similarly-restricted
+          // statements) are rejected by Postgres inside any transaction
+          // block, including one opened by a previous iteration of this
+          // same loop — so this migration gets its own connection and runs
+          // with no surrounding BEGIN/COMMIT at all.
+          const soloClient = await pool.connect();
+          try {
+            await mod.up(soloClient);
+            await soloClient.query(
+              'INSERT INTO schema_migrations (version, name) VALUES ($1, $2)',
+              [m.version, m.name]
+            );
+          } finally {
+            soloClient.release();
           }
+          console.log(`Successfully applied migration (non-transactional): ${m.name}`);
+          continue;
+        }
+
+        await client.query('BEGIN');
+        try {
           await mod.up(client);
           await client.query('INSERT INTO schema_migrations (version, name) VALUES ($1, $2)', [
             m.version,
             m.name,
           ]);
-          console.log(`Successfully applied migration: ${m.name}`);
+          await client.query('COMMIT');
+        } catch (err) {
+          await client.query('ROLLBACK');
+          console.error(`Migration ${m.name} failed, rolled back.`);
+          throw err;
         }
-        await client.query('COMMIT');
-      } catch (err) {
-        await client.query('ROLLBACK');
-        console.error('Migration transaction failed, rolled back changes.');
-        throw err;
+        console.log(`Successfully applied migration: ${m.name}`);
       }
     } else if (action === 'rollback') {
       // Find the highest applied migration
@@ -96,22 +134,34 @@ export async function runMigrations(action: 'up' | 'rollback' = 'up'): Promise<v
       }
 
       console.log(`Rolling back migration: ${m.name}...`);
-      await client.query('BEGIN');
-      try {
-        const filePath = path.join(__dirname, m.filename);
-        const fileUrl = pathToFileURL(filePath).href;
-        const mod = await import(fileUrl);
-        if (typeof mod.down !== 'function') {
-          throw new Error(`Migration ${m.filename} does not export a down function.`);
+      const filePath = path.join(__dirname, m.filename);
+      const fileUrl = pathToFileURL(filePath).href;
+      const mod = (await import(fileUrl)) as MigrationModule;
+      if (typeof mod.down !== 'function') {
+        throw new Error(`Migration ${m.filename} does not export a down function.`);
+      }
+
+      if (mod.nonTransactional) {
+        const soloClient = await pool.connect();
+        try {
+          await mod.down(soloClient);
+          await soloClient.query('DELETE FROM schema_migrations WHERE version = $1', [version]);
+        } finally {
+          soloClient.release();
+        }
+        console.log(`Successfully rolled back migration (non-transactional): ${m.name}`);
+      } else {
+        await client.query('BEGIN');
+        try {
+          await mod.down(client);
+          await client.query('DELETE FROM schema_migrations WHERE version = $1', [version]);
+          await client.query('COMMIT');
+          console.log(`Successfully rolled back migration: ${m.name}`);
+        } catch (err) {
+          await client.query('ROLLBACK');
+          console.error('Rollback transaction failed, rolled back changes.');
+          throw err;
         }
-        await mod.down(client);
-        await client.query('DELETE FROM schema_migrations WHERE version = $1', [version]);
-        await client.query('COMMIT');
-        console.log(`Successfully rolled back migration: ${m.name}`);
-      } catch (err) {
-        await client.query('ROLLBACK');
-        console.error('Rollback transaction failed, rolled back changes.');
-        throw err;
       }
     }
   } finally {