From 2a63a5aadba90152928dbce6fc2d6876006f344f Mon Sep 17 00:00:00 2001 From: Hannah Cheng Date: Fri, 14 Aug 2026 13:33:20 +0800 Subject: [PATCH] chore(migrations): delete 0001, and drop the duplicate indexes it created 0001_add_query_indexes re-declared indexes that GORM's AutoMigrate already creates from index: struct tags on the models. Same columns, different names: AutoMigrate emits "docScope", the migration emitted "idx_restcol_documents_docscope", both over (model_project_id, model_collection_id, created_at). Same for the two collection indexes. Because CREATE INDEX IF NOT EXISTS matches on name and the names differ, its guard never fired: applying it produced a second index over identical columns rather than doing nothing. Measured on a database built by AutoMigrate and then migrated - six indexes where three suffice, costing write throughput and disk for no read benefit. Evidence in FootprintAI/grandturks#986. So 0001 was not merely unapplied, as grandturks#984 concluded from the manifests - it was better left unapplied. Deleting it rather than renaming its indexes to match: a migration that re-declares what the models already declare is a second source of truth for one schema, and the models are the one that services actually run. 0003 removes the duplicates on environments that did apply it. That is not hypothetical: grandturks#987 added a demo-box runner that applies every migrations/*.up.sql in order, so any box brought up on an existing volume since then has them. Only the migration-created names are dropped; the AutoMigrate ones are the real indexes and stay. The two sets are distinguishable because GORM quotes the hyphenated table name where 0001 used underscores. Verified end to end on a real postgres: seeded with AutoMigrate, applied the old 0001 to reproduce the damaged state (6 indexes), applied 0003, confirmed the 3 survivors are AutoMigrate's over the expected columns, and confirmed a second run is a no-op. Numbering is not reused - 0003 follows 0002 - so no environment that recorded a version sees a different migration under an old number. Note for consumers: grandturks copies these files into deploy/demo/restcol-migrations/ with a drift test, so it needs the copy of 0001 removed when it next bumps. Refs: FootprintAI/grandturks#986, FootprintAI/grandturks#984, FootprintAI/grandturks#987 Co-Authored-By: Claude Opus 5 (1M context) --- migrations/0001_add_query_indexes.down.sql | 6 ---- migrations/0001_add_query_indexes.up.sql | 24 --------------- ...0003_drop_redundant_query_indexes.down.sql | 9 ++++++ .../0003_drop_redundant_query_indexes.up.sql | 29 +++++++++++++++++++ migrations/README.md | 22 +++++++++++++- 5 files changed, 59 insertions(+), 31 deletions(-) delete mode 100644 migrations/0001_add_query_indexes.down.sql delete mode 100644 migrations/0001_add_query_indexes.up.sql create mode 100644 migrations/0003_drop_redundant_query_indexes.down.sql create mode 100644 migrations/0003_drop_redundant_query_indexes.up.sql diff --git a/migrations/0001_add_query_indexes.down.sql b/migrations/0001_add_query_indexes.down.sql deleted file mode 100644 index d4416c6..0000000 --- a/migrations/0001_add_query_indexes.down.sql +++ /dev/null @@ -1,6 +0,0 @@ --- 0001_add_query_indexes.down.sql --- Reverts 0001_add_query_indexes.up.sql. - -DROP INDEX IF EXISTS idx_restcol_documents_docscope; -DROP INDEX IF EXISTS idx_restcol_collections_schema_model_collection_id; -DROP INDEX IF EXISTS idx_restcol_collections_model_project_id; diff --git a/migrations/0001_add_query_indexes.up.sql b/migrations/0001_add_query_indexes.up.sql deleted file mode 100644 index 2a9e289..0000000 --- a/migrations/0001_add_query_indexes.up.sql +++ /dev/null @@ -1,24 +0,0 @@ --- 0001_add_query_indexes.up.sql --- Adds the indexes needed to keep ListByProjectID, GetLatestSchema, --- ModelDocument.Query, CountByCollection, and DeleteByCollection off of --- sequential scans as data volume grows. --- --- Applying this migration is only required for environments that run with --- --restcol_auto_migrate=false (production). Dev environments using --- AutoMigrate pick these up from the gorm struct tags on next boot. --- --- All statements are idempotent (IF NOT EXISTS) so it is safe to re-run. - --- ModelCollection.ModelProjectID: backs ListByProjectID. -CREATE INDEX IF NOT EXISTS idx_restcol_collections_model_project_id - ON "restcol-collections" (model_project_id); - --- ModelSchema.ModelCollectionID: backs GetLatestSchema. -CREATE INDEX IF NOT EXISTS idx_restcol_collections_schema_model_collection_id - ON "restcol-collections-schema" (model_collection_id); - --- ModelDocument composite (project, collection, created_at): backs Query, --- CountByCollection, and DeleteByCollection. Leading columns support --- equality filters; trailing created_at supports the ORDER BY in Query. -CREATE INDEX IF NOT EXISTS idx_restcol_documents_docscope - ON "restcol-documents" (model_project_id, model_collection_id, created_at); diff --git a/migrations/0003_drop_redundant_query_indexes.down.sql b/migrations/0003_drop_redundant_query_indexes.down.sql new file mode 100644 index 0000000..aa07285 --- /dev/null +++ b/migrations/0003_drop_redundant_query_indexes.down.sql @@ -0,0 +1,9 @@ +-- 0003_drop_redundant_query_indexes.down.sql +-- +-- Deliberately empty, as with 0002. +-- +-- 0003 drops indexes that duplicate ones AutoMigrate maintains. Recreating them +-- would restore the duplication this migration exists to remove, so a truthful +-- revert does nothing. The indexes that matter were never dropped. + +SELECT 1; diff --git a/migrations/0003_drop_redundant_query_indexes.up.sql b/migrations/0003_drop_redundant_query_indexes.up.sql new file mode 100644 index 0000000..19e5a5e --- /dev/null +++ b/migrations/0003_drop_redundant_query_indexes.up.sql @@ -0,0 +1,29 @@ +-- 0003_drop_redundant_query_indexes.up.sql +-- +-- Removes the indexes 0001_add_query_indexes created, and deletes that +-- migration. They duplicate indexes GORM's AutoMigrate already creates from +-- struct tags on the models: +-- +-- ModelDocument.ModelProjectID index:docScope,priority:1 +-- ModelDocument.ModelCollectionID index:docScope,priority:2 +-- ModelDocument.CreatedAt index:docScope,priority:3 +-- ModelCollection.ModelProjectID index +-- ModelSchema.ModelCollectionID index +-- +-- AutoMigrate emits "docScope"; 0001 emitted "idx_restcol_documents_docscope". +-- Same three columns, different names — so 0001's CREATE INDEX IF NOT EXISTS +-- never matched, and applying it produced a second index over identical +-- columns. Six indexes where three suffice, costing write throughput and disk +-- for no read benefit. Measured, see FootprintAI/grandturks#986. +-- +-- Only the migration-created names are dropped here. The AutoMigrate ones are +-- the real indexes and must stay; the two sets are distinguishable because +-- GORM quotes the hyphenated table name (idx_restcol-collections_...) where +-- 0001 used underscores (idx_restcol_collections_...). +-- +-- A no-op on any environment that never applied 0001, which on the evidence in +-- grandturks#984 is most of them. + +DROP INDEX IF EXISTS idx_restcol_documents_docscope; +DROP INDEX IF EXISTS idx_restcol_collections_model_project_id; +DROP INDEX IF EXISTS idx_restcol_collections_schema_model_collection_id; diff --git a/migrations/README.md b/migrations/README.md index 835f3b7..ee1644f 100644 --- a/migrations/README.md +++ b/migrations/README.md @@ -37,7 +37,7 @@ migrate -path migrations \ `psql` also works for quick one-off application: ```bash -psql "$DATABASE_URL" -f migrations/0001_add_query_indexes.up.sql +psql "$DATABASE_URL" -f migrations/0002_purge_soft_deleted_documents.up.sql ``` ## Writing a new migration @@ -58,3 +58,23 @@ baseline/snapshot migration matching the current live schema, and deciding how to handle the existing AutoMigrate state. Track that as a separate project. [golang-migrate]: https://github.com/golang-migrate/migrate + +## What is here, and what is deliberately not + +| | | +|---|---| +| `0002_purge_soft_deleted_documents` | erases document rows tombstoned before DeleteDocument became a hard delete (#137). Irreversible; its down migration is a documented no-op | +| `0003_drop_redundant_query_indexes` | removes the indexes `0001` used to create | + +**There is no `0001`.** It added query indexes that GORM's `AutoMigrate` already +creates from `index:` struct tags on the models — the same columns under +different names, so its `CREATE INDEX IF NOT EXISTS` never matched and applying +it produced a second index over each. It was deleted rather than fixed: a +migration that re-declares what the models already declare is a second source of +truth for the same schema, and the models win. + +Do not re-add index migrations for indexes struct tags can express. Add a +migration when the change is one struct tags *cannot* describe — a backfill, a +data purge, an index built concurrently, a column rewrite. + +Numbering is not reused: `0003` follows `0002` even though `0001` is gone.