From a92fad9796f96b3df195b491bb25f7489e33280b Mon Sep 17 00:00:00 2001 From: Jared Tobin Date: Thu, 2 Apr 2026 16:13:02 +0800 Subject: [PATCH 1/2] tapdb: fix Postgres sequence desyncs Migration 31 copied universe_leaves rows with explicit ids, leaving the BIGSERIAL sequence behind the actual data. New inserts then hit duplicate-key errors. Add migration 55 to reset the universe_leaves sequence via setval(). Also reset supply_commit_states and supply_commit_update_types (migration 40 inserted explicit ids into these enum tables); these are benign since no code path auto-increments into them, but fixing them keeps sequence state consistent. On SQLite the statements are replaced with no-ops. --- tapdb/migrations.go | 2 +- .../000055_fix_universe_leaves_seq.down.sql | 2 ++ .../000055_fix_universe_leaves_seq.up.sql | 11 +++++++++++ tapdb/sqlite.go | 18 +++++++++++++++--- 4 files changed, 29 insertions(+), 4 deletions(-) create mode 100644 tapdb/sqlc/migrations/000055_fix_universe_leaves_seq.down.sql create mode 100644 tapdb/sqlc/migrations/000055_fix_universe_leaves_seq.up.sql diff --git a/tapdb/migrations.go b/tapdb/migrations.go index 0f0981e7b..b316711de 100644 --- a/tapdb/migrations.go +++ b/tapdb/migrations.go @@ -24,7 +24,7 @@ const ( // daemon. // // NOTE: This MUST be updated when a new migration is added. - LatestMigrationVersion = 54 + LatestMigrationVersion = 55 ) // DatabaseBackend is an interface that contains all methods our different diff --git a/tapdb/sqlc/migrations/000055_fix_universe_leaves_seq.down.sql b/tapdb/sqlc/migrations/000055_fix_universe_leaves_seq.down.sql new file mode 100644 index 000000000..a1af64ac2 --- /dev/null +++ b/tapdb/sqlc/migrations/000055_fix_universe_leaves_seq.down.sql @@ -0,0 +1,2 @@ +-- Sequence reset is forward-only; no rollback needed. +SELECT 1; diff --git a/tapdb/sqlc/migrations/000055_fix_universe_leaves_seq.up.sql b/tapdb/sqlc/migrations/000055_fix_universe_leaves_seq.up.sql new file mode 100644 index 000000000..29abb6e93 --- /dev/null +++ b/tapdb/sqlc/migrations/000055_fix_universe_leaves_seq.up.sql @@ -0,0 +1,11 @@ +-- Reset BIGSERIAL sequences that were left behind by migrations +-- that inserted rows with explicit id values. +-- +-- Migration 31 copied universe_leaves rows with explicit ids. +-- Migration 40 inserted supply_commit_states (0-6) and +-- supply_commit_update_types (0-2) with explicit ids. +-- +-- On SQLite these are no-ops (replaced via sqliteSchemaReplacements). +SELECT setval(pg_get_serial_sequence('universe_leaves', 'id'), COALESCE((SELECT MAX(id) FROM universe_leaves), 0)); +SELECT setval(pg_get_serial_sequence('supply_commit_states', 'id'), COALESCE((SELECT MAX(id) FROM supply_commit_states), 0)); +SELECT setval(pg_get_serial_sequence('supply_commit_update_types', 'id'), COALESCE((SELECT MAX(id) FROM supply_commit_update_types), 0)); diff --git a/tapdb/sqlite.go b/tapdb/sqlite.go index 888b64b61..aeb45b70c 100644 --- a/tapdb/sqlite.go +++ b/tapdb/sqlite.go @@ -39,9 +39,21 @@ const ( var ( // sqliteSchemaReplacements is a map of schema strings that need to be - // replaced for sqlite. There currently aren't any replacements, because - // the SQL files are written with SQLite compatibility in mind. - sqliteSchemaReplacements = map[string]string{} + // replaced for sqlite. + sqliteSchemaReplacements = map[string]string{ + "SELECT setval(pg_get_serial_sequence(" + + "'universe_leaves', 'id'), COALESCE((" + + "SELECT MAX(id) FROM universe_leaves" + + "), 0));": "SELECT 1;", + "SELECT setval(pg_get_serial_sequence(" + + "'supply_commit_states', 'id'), " + + "COALESCE((SELECT MAX(id) FROM " + + "supply_commit_states), 0));": "SELECT 1;", + "SELECT setval(pg_get_serial_sequence(" + + "'supply_commit_update_types', 'id'), " + + "COALESCE((SELECT MAX(id) FROM " + + "supply_commit_update_types), 0));": "SELECT 1;", + } ) // SqliteConfig holds all the config arguments needed to interact with our From 4c3089e9154dcdcce1f7fc0b58f68fc6bbce3b03 Mon Sep 17 00:00:00 2001 From: Jared Tobin Date: Thu, 2 Apr 2026 16:13:06 +0800 Subject: [PATCH 2/2] tapdb: add generic Postgres sequence consistency test Add TestSequenceConsistency, a Postgres-only test that verifies every BIGSERIAL sequence is consistent with its table's max ID after all migrations have run. This catches migrations that recreate tables and copy rows with explicit IDs without advancing the sequence. --- tapdb/migrations_test.go | 120 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 120 insertions(+) diff --git a/tapdb/migrations_test.go b/tapdb/migrations_test.go index 2301d3fec..bf8f005cb 100644 --- a/tapdb/migrations_test.go +++ b/tapdb/migrations_test.go @@ -1279,3 +1279,123 @@ func TestMigration54Down(t *testing.T) { "delete should fail after reverting CASCADE", ) } + +// TestSequenceConsistency is a Postgres-only test that verifies every +// auto-increment sequence is consistent with its table's max ID after +// all migrations have run. This catches migrations that copy rows with +// explicit IDs (e.g. INSERT INTO new_t (id, ...) SELECT id, ... FROM +// old_t) without advancing the sequence. +func TestSequenceConsistency(t *testing.T) { + ctx := context.Background() + + db := NewTestDBWithVersion(t, 30) + + if db.Backend() != sqlc.BackendTypePostgres { + t.Skip("sequence consistency only applies to Postgres") + } + + // Seed a universe_leaves row with an explicit id = 100. + // Migration 31 will copy this row (with its explicit ID) + // into a recreated table, which won't advance the sequence. + InsertTestdata( + t, db.BaseDB, + "migrations_test_00031_dummy_data.sql", + ) + + // Reset every sequence to match its table's current max so + // that only desyncs introduced by migrations are detected. + _, err := db.ExecContext(ctx, ` + DO $$ + DECLARE + r RECORD; + BEGIN + FOR r IN + SELECT + seq.relname AS seq_name, + tbl.relname AS tbl_name, + a.attname AS col_name + FROM pg_class seq + JOIN pg_depend d + ON d.objid = seq.oid + JOIN pg_class tbl + ON d.refobjid = tbl.oid + JOIN pg_attribute a + ON a.attrelid = tbl.oid + AND a.attnum = d.refobjsubid + WHERE seq.relkind = 'S' + AND d.deptype = 'a' + LOOP + EXECUTE format( + 'SELECT setval(%L, COALESCE(' + || '(SELECT MAX(%I) FROM %I),' + || ' 1))', + r.seq_name, r.col_name, + r.tbl_name + ); + END LOOP; + END + $$; + `) + require.NoError(t, err) + + // Run all remaining migrations. + err = db.ExecuteMigrations(TargetLatest) + require.NoError(t, err) + + // Find all sequences owned by table columns. + rows, err := db.QueryContext(ctx, ` + SELECT + seq.relname AS sequence_name, + tbl.relname AS table_name, + a.attname AS column_name + FROM pg_class seq + JOIN pg_depend d ON d.objid = seq.oid + JOIN pg_class tbl ON d.refobjid = tbl.oid + JOIN pg_attribute a ON a.attrelid = tbl.oid + AND a.attnum = d.refobjsubid + WHERE seq.relkind = 'S' + AND d.deptype = 'a' + `) + require.NoError(t, err) + + defer rows.Close() + + type seqInfo struct { + seqName string + table string + column string + } + + var sequences []seqInfo + for rows.Next() { + var s seqInfo + err := rows.Scan(&s.seqName, &s.table, &s.column) + require.NoError(t, err) + sequences = append(sequences, s) + } + require.NoError(t, rows.Err()) + require.NotEmpty(t, sequences, "expected at least one sequence") + + for _, s := range sequences { + var lastValue int64 + err := db.QueryRowContext(ctx, fmt.Sprintf( + "SELECT last_value FROM %s", s.seqName, + )).Scan(&lastValue) + require.NoError(t, err) + + var maxID int64 + err = db.QueryRowContext(ctx, fmt.Sprintf( + "SELECT COALESCE(MAX(%s), 0) FROM %s", + s.column, s.table, + )).Scan(&maxID) + require.NoError(t, err) + + require.GreaterOrEqual( + t, lastValue, maxID, + "sequence %s on %s.%s is behind: "+ + "last_value=%d, max(%s)=%d", + s.seqName, s.table, s.column, + lastValue, s.column, maxID, + ) + } +}