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.View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.