diff --git a/server/src/at_record_server/shelf_index_postgres.gleam b/server/src/at_record_server/shelf_index_postgres.gleam index e22cd28..867ae5d 100644 --- a/server/src/at_record_server/shelf_index_postgres.gleam +++ b/server/src/at_record_server/shelf_index_postgres.gleam @@ -13,6 +13,7 @@ import at_record_server/shelf_index.{ SeenOps, ShelfEntryEvent, Store, } import gleam/dynamic/decode +import gleam/int import gleam/option.{type Option, None, Some} import gleam/result import pog @@ -45,93 +46,144 @@ pub fn table_store(conn: pog.Connection) -> Result(shelf_index.Store, String) { } fn migrate(conn: pog.Connection) -> Result(Nil, String) { - use _ <- result.try( - pog.query( - "create table if not exists shelf_entry_events ( - event_uri text primary key, - entry_uri text not null, - did text not null, - rkey text not null, - action text not null, - created_at text not null, - record_json text not null, - indexed_at text not null - )", - ) - |> pog.execute(conn) - |> result.replace_error("shelf_entry_events migration failed"), - ) - use _ <- result.try( - pog.query( - "create index if not exists shelf_entry_events_entry_rkey_idx - on shelf_entry_events (entry_uri, rkey)", - ) - |> pog.execute(conn) - |> result.replace_error("shelf_entry_events index migration failed"), - ) - use _ <- result.try( - pog.query( - "create table if not exists shelf_entries ( - entry_uri text primary key, - did text not null, - status text not null, - title text, - artist_display text, - year int, - format text, - thumb_url text, - cover_cid text, - media_grade text, - sleeve_grade text, - rating int, - folder text, - notes text, - release_uri text, - release_cid text, - price_amount int, - price_currency text, - counterparty text, - source_client_agent text, - source_origin text, - source_origin_url text, - source_external_provider text, - source_external_id text, - source_external_url text, - source_record_uri text, - source_record_cid text, - created_at text not null, - updated_at text not null, - indexed_at text not null - )", - ) - |> pog.execute(conn) - |> result.replace_error("shelf_entries migration failed"), - ) - use _ <- result.try( - pog.query( - "create index if not exists shelf_entries_did_status_idx - on shelf_entries (did, status)", - ) - |> pog.execute(conn) - |> result.replace_error("shelf_entries did/status index migration failed"), - ) - use _ <- result.try( - pog.query( - "create index if not exists shelf_entries_release_uri_idx - on shelf_entries (release_uri)", - ) - |> pog.execute(conn) - |> result.replace_error("shelf_entries release_uri index migration failed"), - ) - pog.query( + use _ <- result.try(migration_step( + conn, + "create table if not exists shelf_entry_events ( + event_uri text primary key, + entry_uri text not null, + did text not null, + rkey text not null, + action text not null, + created_at text not null, + record_json text not null, + indexed_at text not null + )", + "shelf_entry_events migration failed", + )) + use _ <- result.try(migration_step( + conn, + "create index if not exists shelf_entry_events_entry_rkey_idx + on shelf_entry_events (entry_uri, rkey)", + "shelf_entry_events index migration failed", + )) + use _ <- result.try(migration_step( + conn, + "create table if not exists shelf_entries ( + entry_uri text primary key, + did text not null, + status text not null, + title text, + artist_display text, + year int, + format text, + thumb_url text, + cover_cid text, + media_grade text, + sleeve_grade text, + rating int, + folder text, + notes text, + release_uri text, + release_cid text, + price_amount int, + price_currency text, + counterparty text, + source_client_agent text, + source_origin text, + source_origin_url text, + source_external_provider text, + source_external_id text, + source_external_url text, + source_record_uri text, + source_record_cid text, + created_at text not null, + updated_at text not null, + indexed_at text not null + )", + "shelf_entries migration failed", + )) + use _ <- result.try(migration_step( + conn, + "create index if not exists shelf_entries_did_status_idx + on shelf_entries (did, status)", + "shelf_entries did/status index migration failed", + )) + use _ <- result.try(migration_step( + conn, + "create index if not exists shelf_entries_release_uri_idx + on shelf_entries (release_uri)", + "shelf_entries release_uri index migration failed", + )) + migration_step( + conn, "create table if not exists shelf_index_seen ( did text primary key, seen_at text not null )", + "shelf_index_seen migration failed", ) - |> pog.execute(conn) - |> result.replace_error("shelf_index_seen migration failed") - |> result.map(fn(_) { Nil }) +} + +/// Runs one migration statement, treating a lost `if not exists` race (see +/// `is_race_loser`) as success and anything else as a labeled failure. +fn migration_step( + conn: pog.Connection, + sql: String, + label: String, +) -> Result(Nil, String) { + case pog.query(sql) |> pog.execute(conn) { + Ok(_) -> Ok(Nil) + Error(e) -> + case is_race_loser(e) { + True -> Ok(Nil) + False -> Error(label <> ": " <> describe_query_error(e)) + } + } +} + +/// `create table/index if not exists` is not actually safe under concurrent +/// callers (every `with_store`-backed test opens its own fresh pool and +/// re-runs `migrate`): two sessions can both pass the "not exists" check +/// before either commits, and the loser then fails inserting its own system +/// catalog row -- a duplicate key on `pg_type_typname_nsp_index` (a table) +/// or `pg_class_relname_nsp_index` (an index), not anything +/// migration-specific. Confirmed locally: five concurrent `create table if +/// not exists shelf_entry_events` statements against a real +/// non-superuser-owned Postgres 18 database, four of five failed with +/// exactly this. Losing this specific race means the object exists now, +/// which was always the actual goal, so it is not a real failure. +fn is_race_loser(e: pog.QueryError) -> Bool { + case e { + pog.ConstraintViolated(constraint:, ..) -> + constraint == "pg_type_typname_nsp_index" + || constraint == "pg_class_relname_nsp_index" + _ -> False + } +} + +fn describe_query_error(e: pog.QueryError) -> String { + case e { + pog.ConstraintViolated(message:, constraint:, detail:) -> + "constraint " + <> constraint + <> " violated: " + <> message + <> " (" + <> detail + <> ")" + pog.PostgresqlError(code:, name:, message:) -> + "postgres error " <> code <> " (" <> name <> "): " <> message + pog.UnexpectedArgumentCount(expected:, got:) -> + "expected " + <> int.to_string(expected) + <> " argument(s), got " + <> int.to_string(got) + pog.UnexpectedArgumentType(expected:, got:) -> + "expected argument type " <> expected <> ", got " <> got + pog.UnexpectedResultType(_) -> "unexpected result shape" + pog.QueryTimeout -> "query timed out" + pog.ConnectionUnavailable -> "no connection available" + } } fn upsert_event(conn: pog.Connection, event: ShelfEntryEvent) -> Nil {