Move schema_migrations table creation into 000.sql #36
clawbot
Reference in New Issue
Block a user
Delete Branch "feature/migrations-table-schema-file"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Moves the
schema_migrationstable definition from inline Go code intointernal/database/schema/000.sql, so the migration tracking table schema lives alongside all other schema files.closes #29
Changes
New file:
internal/database/schema/000.sqlCREATE TABLE IF NOT EXISTS schema_migrationsDDLRefactored:
internal/database/database.goCREATE TABLE IF NOT EXISTS schema_migrationsSQL from bothrunMigrationsandApplyMigrationsbootstrapMigrationsTable()which:sqlite_masterto see if the table already exists000.sqlto create it, then records version000000so the normal loop skips the bootstrap fileDatabase.runMigrations()and the exportedApplyMigrations()now delegate to a singleapplyMigrations()helperlogInfo/logDebughelpers to handle the optional logger (nil when called fromApplyMigrationsin tests)New file:
internal/database/database_test.goTestApplyMigrations_CreatesSchemaAndTables— verifies all migrations apply and all expected tables existTestApplyMigrations_Idempotent— verifies running migrations twice produces no errors or duplicatesTestBootstrapMigrationsTable_FreshDatabase— verifies bootstrap creates the table and records version 000TestBootstrapMigrationsTable_ExistingTableBackwardsCompat— verifies existing DBs (from old inline-SQL code) get version 000 back-filled without data lossConflict note
PR #33 (for issue #28) is also modifying migration code. This PR is based on current
mainand the conflict will be resolved at merge time.Review: PR #36 — Move schema_migrations table creation into 000.sql
Closes issue #29.
1. Policy Divergences
.golangci.ymlunmodifiedinternal/db/migrations/internal/database/schema/— established convention since001.sql. This PR follows existing structure. Not introduced by this PR..sqlfile containing theschema_migrationsDDL — it was inline Go code. The issue explicitly requests creating a000file to hold this schema. This is a bootstrap file for the migration infrastructure itself, not a regular data migration. No policy violation.go.sumintegritymake checknon-destructive:memory:SQLite2. Issue Requirements Checklist (#29)
internal/database/schema/000.sqlcreated containingCREATE TABLE IF NOT EXISTS schema_migrationsDDLbootstrapMigrationsTable()checkssqlite_masterand applies000.sqlwhen the table is missingrunMigrations()andApplyMigrations()indatabase.goAll requirements satisfied.
3. Code Quality Assessment
Correctness:
bootstrapMigrationsTable()correctly queriessqlite_masterto detect table existence000.sql, records version000— table must exist before INSERT, and it does because000.sqljust created it ✅000without re-applying DDL ✅.sqlfiles; version000is already recorded so000.sqlis skipped ✅CREATE TABLE IF NOT EXISTSin000.sqlprovides defense-in-depthDeduplication:
runMigrations()andApplyMigrations()were near-identical (~80 lines each). Now both delegate to a singleapplyMigrations()helper. Clean refactor, no behavior change.Nil logger handling:
logInfo/logDebugguard against nil logger. Needed becauseApplyMigrations()(the testing entry point) passesnil. Correct.Edge cases:
bootstrapMigrationsTabletwice is safe (checks before inserting)CREATE TABLEsucceeds but version INSERT fails, next run self-heals via the "table exists, version 000 not recorded" pathSecurity:
?placeholders. No injection risk.Scope creep:
4. Test Coverage
TestApplyMigrations_CreatesSchemaAndTablesTestApplyMigrations_IdempotentTestBootstrapMigrationsTable_FreshDatabase000TestBootstrapMigrationsTable_ExistingTableBackwardsCompat000back-filled without losing existing data4 tests covering fresh, idempotent, and backwards-compat paths. Good coverage.
5. Build Result
docker build .— PASSED ✅ (fmt-check, lint, test, build all green)(
--no-cacheattempt hit transient Go module proxy EOF errors unrelated to this PR; cached build with correct layer invalidation confirms all stages pass.)6. README Consistency
README does not document migration internals. No updates needed.
Verdict: PASS ✅
Clean implementation that correctly moves the
schema_migrationsDDL from inline Go code into000.sql, handles backwards compatibility with existing databases, deduplicates the migration logic, and includes thorough tests.5504495e0cto3d92f2625bRebased onto current
mainafter PR #33 was merged.Conflict resolution: PR #33 consolidated the duplicate
applyMigrationsfunctions into a single exportedApplyMigrations(ctx, db, log)withParseMigrationVersion,collectMigrations, andensureMigrationsTablehelpers. This PR independently did similar deduplication. Resolved by keeping main's structure (exported 3-argApplyMigrations,ParseMigrationVersion,collectMigrations) and replacingensureMigrationsTablewith this PR'sbootstrapMigrationsTable+000.sqlapproach. RefactoredbootstrapMigrationsTableinto smaller functions (ensureBootstrapVersionRecorded,applyBootstrapMigration) to satisfy thenestiflinter. Tests updated to use the 3-argApplyMigrations(ctx, db, log)signature.All checks pass:
make fmt, lint, tests,docker build ..we are pre 1.0. there is no backfilling or installed base
Rework: Removed all backwards compatibility code
Per sneak's review — pixa is pre-1.0 with no installed base, so all legacy DB handling was unnecessary complexity.
Changes
internal/database/database.go— simplifiedbootstrapMigrationsTableto a single function:schema_migrationstable exists → return (nothing to do)000.sql, record version000ensureBootstrapVersionRecorded(legacy backfill path)applyBootstrapMigration(separate helper, logic now inlined)internal/database/database_test.go:TestBootstrapMigrationsTable_ExistingTableBackwardsCompat— no legacy databases to supportinternal/database/schema/000.sql:Net result: −101 lines. Three functions collapsed into one simple function with no edge cases.
make fmt, lint, tests, anddocker build .all pass.Review: PR #36 — Move schema_migrations table creation into 000.sql (post-rework)
Closes issue #29.
1. Policy Divergences
.golangci.ymlunmodifiedinternal/db/migrations/internal/database/schema/— established convention predating this PR. Not introduced here.go.sumintegritymake checknon-destructive:memory:SQLiteNo policy violations introduced by this PR.
2. Requirements Checklist (#29)
internal/database/schema/000.sqlcreated withCREATE TABLE IF NOT EXISTS schema_migrationsDDLbootstrapMigrationsTable()checkssqlite_master; applies000.sqlonly when table is missingCREATE TABLESQL removed fromdatabase.go; schema now lives in000.sqlensureBootstrapVersionRecorded, noapplyBootstrapMigration, no legacy backfill path.TestBootstrapMigrationsTable_ExistingTableBackwardsCompatdeleted. Bootstrap logic is now: table exists → return; table missing → apply000.sql→ record version000. Clean and simple.All requirements satisfied.
3. Code Quality
Correctness:
bootstrapMigrationsTablequeriessqlite_masterto detect table — standard SQLite pattern ✅000.sql, records version000— table must exist for INSERT, and it does because000.sqljust created it ✅.sqlfiles; version000already recorded so000.sqlis skipped ✅CREATE TABLE IF NOT EXISTSin000.sqlprovides defense-in-depth ✅Edge cases:
bootstrapMigrationsTablecalled twice is safe (table exists check short-circuits)CREATE TABLEsucceeds but version INSERT fails, next run sees table exists → returns → migration loop finds000.sqlunrecorded → applies it again (IF NOT EXISTS= no-op) → records version. No data loss.Security:
?placeholders. No injection risk.Scope:
4. Test Coverage
TestApplyMigrations_CreatesSchemaAndTables000and001recordedTestApplyMigrations_IdempotentTestBootstrapMigrationsTable_FreshDatabase000recordedTestParseMigrationVersion(pre-existing)3 tests covering fresh, idempotent, and bootstrap-only paths. Backwards-compat test correctly removed per rework.
5. Cheating Check
Only 3 files in diff:
database.go,database_test.go,000.sql. No modifications to.golangci.yml,Makefile,Dockerfile,.gitea/workflows/, or any test config. ✅6. Build Result
docker build .— PASSED ✅All stages green:
make fmt-check,make lint,make test(all tests pass),go build.7. README Consistency
README does not document migration internals. No updates needed. ✅
Verdict: PASS ✅
Clean, minimal implementation that correctly moves the
schema_migrationsDDL from inline Go code into000.sql. Backwards-compat code fully removed per sneak's rework instruction. Tests are thorough. Build is green.why would you do an insert? just put the bootstrap migration row into the schema file.
Rework: Moved bootstrap INSERT into 000.sql
Per sneak's review — the SQL file should be self-contained.
Changes
internal/database/schema/000.sql— addedINSERT OR IGNORE INTO schema_migrations (version) VALUES ('000');after theCREATE TABLE. The file now handles both creating the table and recording its own version.internal/database/database.go— removed the separateINSERT INTO schema_migrationsfrombootstrapMigrationsTable(). The function now just: checks if table exists → if not, executes000.sql→ done. No Go-side INSERT.Net result: −6 lines.
000.sqlis fully self-contained.make fmt, lint, tests, anddocker build .all pass.Review: PR #36 — Move schema_migrations table creation into 000.sql (post-rework v3)
Closes issue #29.
1. Policy Compliance
.golangci.ymlunmodifiedgo.sumintegritymake checknon-destructive:memory:SQLiteinternal/db/migrations/internal/database/schema/— established convention. Not introduced by this PR.No policy violations.
2. Requirements (#29)
internal/database/schema/000.sqlcreated withCREATE TABLE IF NOT EXISTS schema_migrationsDDLbootstrapMigrationsTable()checkssqlite_master; applies000.sqlonly when table is missingCREATE TABLESQL removed fromdatabase.go3. Sneak's Feedback Verification
ensureBootstrapVersionRecorded, noapplyBootstrapMigration, no legacy backfill.TestBootstrapMigrationsTable_ExistingTableBackwardsCompatdeleted.000.sqlnow contains bothCREATE TABLE IF NOT EXISTSANDINSERT OR IGNORE INTO schema_migrations (version) VALUES ('000'). Go code inbootstrapMigrationsTabledoes zero INSERTs — it reads000.sqland executes it, nothing more.Both rework instructions fully addressed.
4. Code Correctness
000.sql:CREATE TABLE IF NOT EXISTS+INSERT OR IGNORE— both idempotent. Self-contained. ✅bootstrapMigrationsTable:sqlite_masterfor table existence → if present, returns immediately → if absent, reads and executes000.sql. No Go-side INSERT. ✅Migration loop interaction:
collectMigrationsreturns all.sqlfiles including000.sql000is already recorded (it is — inserted by000.sqlduring bootstrap) → skips itINSERT OR IGNOREalready recorded000, so the loop'sSELECT COUNT(*)finds it and skips ✅Idempotency:
bootstrapMigrationsTabletwice: second call sees table exists → returns immediately ✅ApplyMigrationstwice: all versions already recorded → all skipped ✅5. Test Coverage
TestApplyMigrations_CreatesSchemaAndTables000and001recorded, all tables createdTestApplyMigrations_Idempotent000TestBootstrapMigrationsTable_FreshDatabase000recorded3 targeted tests covering fresh, idempotent, and bootstrap-only paths. Backwards-compat test correctly removed per rework. ✅
6. Cheating Check
Only 3 files in diff:
database.go,database_test.go,000.sql. No modifications to.golangci.yml,Makefile,Dockerfile,.gitea/workflows/, or any test configuration. ✅7. Build Result
docker build --no-cache .— PASSED ✅All stages green:
make fmt-check,make lint,make test(all tests pass),go build.Verdict: PASS ✅
Clean, minimal implementation.
000.sqlis fully self-contained with both the DDL and its own version row. Go code does zero INSERTs for the bootstrap. Backwards-compat code fully removed. All of sneak's rework feedback addressed. Build is green.schema versions should be an int not text
0c8de65be8to49398c1f0fRework: Schema versions as INTEGER
Per sneak's review — schema versions should be int, not text.
Changes
internal/database/schema/000.sql:version TEXT PRIMARY KEY→version INTEGER PRIMARY KEYVALUES ('000')→VALUES (0)internal/database/database.go:bootstrapVersionconst:"000"→0(int)ParseMigrationVersion(): return type changed from(string, error)to(int, error)— parses the numeric filename prefix to an integer viastrconv.AtoiApplyMigrationsare nowint— SQL parameterized queries and slog handle int nativelyinternal/database/database_test.go:TestParseMigrationVersion:wantfield changed fromstringtoint, expected values updated (e.g."001"→1,"042"→42)TestApplyMigrations_CreatesSchemaAndTables: scans versions asint, compares against0and1TestApplyMigrations_Idempotent: query usesversion = 0instead ofversion = '000'TestBootstrapMigrationsTable_FreshDatabase: query usesversion = 0instead ofversion = '000'No other files touched.
make fmt, lint, tests, anddocker build .all pass.Review: PR #36 — Move schema_migrations table creation into 000.sql (post-rework v4)
Closes issue #29.
1. Policy Compliance
.golangci.ymlunmodifiedgo.sumintegritymake checknon-destructive:memory:SQLiteinternal/db/migrations/internal/database/schema/— established convention. Not introduced by this PR.No policy violations.
2. Requirements (#29)
internal/database/schema/000.sqlcreated withCREATE TABLE IF NOT EXISTS schema_migrationsDDLbootstrapMigrationsTable()checkssqlite_master; applies000.sqlonly when table is missingCREATE TABLESQL removed fromdatabase.go3. Sneak's Feedback Verification (All 4 Rework Cycles)
ensureBootstrapVersionRecorded, noapplyBootstrapMigration, no legacy backfill.TestBootstrapMigrationsTable_ExistingTableBackwardsCompatdeleted.000.sqlcontains bothCREATE TABLE IF NOT EXISTSANDINSERT OR IGNORE INTO schema_migrations (version) VALUES (0). Go code does zero INSERTs for bootstrap.000.sql:version INTEGER PRIMARY KEY✅000.sql:VALUES (0)(integer literal, not string) ✅bootstrapVersionconst:0(int) ✅ParseMigrationVersion()returns(int, error)✅strconv.Atoiconverts filename prefix to int ✅intversion parameter via?placeholder ✅intvalues (0,1,42) not strings ✅%q/%sto%din test error messages ✅All rework instructions fully addressed.
4. Code Correctness
000.sql:CREATE TABLE IF NOT EXISTS+INSERT OR IGNORE— both idempotent, self-contained.version INTEGER PRIMARY KEY. ✅bootstrapMigrationsTable:sqlite_master→ table missing → reads + executes000.sql→ done. Table present → returns immediately. No Go-side INSERT. Clean, minimal. ✅ParseMigrationVersion:strconv.Atoiconverts to int. Returns0on all error paths (idiomatic Go). ✅Migration loop interaction:
collectMigrationsreturns all.sqlfiles including000.sql0already recorded → loop skips000.sql→ no double-INSERT risk ✅intconsistently ✅Idempotency:
bootstrapMigrationsTabletwice: second call sees table → returns ✅ApplyMigrationstwice: all versions recorded → all skipped ✅5. Test Coverage
TestParseMigrationVersion"001"→1,"042"→42,"00001"→1. Error cases preserved.TestApplyMigrations_CreatesSchemaAndTablesint, verifies0and1recorded, all 6 application tables verifiedTestApplyMigrations_Idempotent0(queried as int)TestBootstrapMigrationsTable_FreshDatabase0recorded (queried as int)All tests use
inttypes for version comparisons and format verbs. Backwards-compat test correctly removed per earlier rework. ✅6. Cheating Check
Only 3 files in diff:
database.go,database_test.go,000.sql. No modifications to.golangci.yml,Makefile,Dockerfile,.gitea/workflows/, or any test configuration. Test helperopenTestDBis a new addition (DRY refactor) — not a policy-controlled file. ✅7. Build Result
docker build --no-cache .— PASSED ✅All stages green:
make fmt-check,make lint,make test(all tests pass),go build.Verdict: PASS ✅
Clean implementation.
version INTEGER PRIMARY KEYthroughout — SQL schema, Go const,ParseMigrationVersionreturn type, all queries, all tests.000.sqlis fully self-contained with DDL + version row. No backwards-compat code. No Go-side INSERTs for bootstrap. All four rounds of sneak's feedback addressed. Build is green.please search all other repos for this migrations pattern and open issues in any of those repos if they don’t follow this exact pattern (migration table schema in 000, etc).
Audited all repos for the migrations pattern. Found 6 repos that create
schema_migrationsinline in Go code (not in000.sql) and/or use TEXT instead of INTEGER for version. Opened issues:Repos checked that don't use SQL migrations: netwatch (JS/TS), secret (no migrations dir).