fix: ensure schema exists before migrate and cleanAndMigrate#3
Merged
Conversation
Migrator.cleanAndMigrate() and Migrator.migrate() failed unrecoverably when the configured schema did not exist: isSchemaEmpty conflated "schema empty" with "schema missing", so cleanAndMigrate skipped the recreate branch and migrate() then issued an unqualified CREATE TYPE that Postgres rejected with "no schema has been selected to create in". Establish the invariant that the configured schema must exist before any entry point runs. A new autoCreateSchema: Boolean = true constructor flag drives CREATE SCHEMA IF NOT EXISTS at the start of migrate() and cleanAndMigrate(); set to false for least-privilege roles lacking the CREATE privilege. A secondary 5-arg constructor preserves binary compatibility with the 0.1.0 signature for Java callers, pre-compiled binaries, and reflective callers using the old arity. Reflection in NomadPlugin now selects the primary constructor by maximum arity so it remains deterministic with both constructors present. The sbt plugin gains nomadSchema (default "public") and nomadAutoCreateSchema (default true) settings, achieving parity with the library constructor instead of hard-coding values. Scripted test coverage extended on H2 and Postgres: Test 7 asserts the exact PG error surfaces when the opt-out is used (pinning the original bug symptom), and a canonical Test 9 replays the verbatim repro from the bug report (DROP SCHEMA public CASCADE then cleanAndMigrate) plus a second invocation proving the fix is idempotent against the self-perpetuating nature of the original failure.
There was a problem hiding this comment.
Pull request overview
This PR addresses a migration failure mode where Migrator.migrate() / cleanAndMigrate() could get stuck when the configured schema is missing, by optionally auto-creating the schema up front and wiring schema configuration through the sbt plugin.
Changes:
- Add
autoCreateSchema(defaulttrue) toMigrator, ensuring schema creation beforemigrate()/cleanAndMigrate()(with a 5-arg constructor preserved for binary compatibility). - Add sbt plugin settings
nomadSchemaandnomadAutoCreateSchema, and pass them intoMigratorvia reflection. - Expand scripted tests for H2 and Postgres to cover “missing schema self-heal” and the opt-out behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
version.sbt |
Bumps snapshot version to 0.1.1-SNAPSHOT. |
core/src/main/scala/nomad/Migrator.scala |
Introduces autoCreateSchema and schema creation before entry points. |
sbt-plugin/src/main/scala/nomad/sbt/NomadPlugin.scala |
Adds schema/autocreate settings and forwards them to Migrator. |
sbt-plugin/src/sbt-test/nomad/migrate-custom-schema/src/main/scala/Main.scala |
Adds H2 scripted assertions for missing-schema self-heal and opt-out failure. |
sbt-plugin/src/sbt-test/nomad/clean-and-migrate/src/main/scala/Main.scala |
Adds H2 scripted assertions for cleanAndMigrate self-heal and opt-out failure. |
sbt-plugin/src/sbt-test/nomad/clean-and-migrate-postgres/src/main/scala/Main.scala |
Adds Postgres scripted coverage for missing-schema self-heal + canonical repro + opt-out behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
This PR prevents Migrator.migrate() and Migrator.cleanAndMigrate() from failing when the configured schema does not yet exist by optionally auto-creating the schema up front, and wires schema/auto-create configurability through the sbt plugin with regression coverage (H2 + Postgres).
Changes:
- Add
autoCreateSchema: Boolean = truetoMigratorand runCREATE SCHEMA IF NOT EXISTSat the start ofmigrate()andcleanAndMigrate()(with an auxiliary constructor to preserve the 0.1.0 5-arg signature). - Add sbt plugin settings
nomadSchemaandnomadAutoCreateSchema, and pass them when reflectively constructingMigrator. - Extend scripted tests to cover “missing schema self-heals” and “opt-out reproduces original failure” for H2 and Postgres (including the canonical
DROP SCHEMA public CASCADErepro).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
core/src/main/scala/nomad/Migrator.scala |
Introduces autoCreateSchema and ensureSchemaExists, invoked at the start of migrate() / cleanAndMigrate(), plus auxiliary ctor for binary compatibility. |
sbt-plugin/src/main/scala/nomad/sbt/NomadPlugin.scala |
Adds nomadSchema / nomadAutoCreateSchema settings and passes them into reflective Migrator construction (constructor selected deterministically). |
sbt-plugin/src/sbt-test/nomad/clean-and-migrate/src/main/scala/Main.scala |
Adds H2 assertions for missing-schema self-heal and opt-out failure behavior for cleanAndMigrate(). |
sbt-plugin/src/sbt-test/nomad/migrate-custom-schema/src/main/scala/Main.scala |
Adds H2 assertions for missing-schema self-heal and opt-out failure behavior for migrate(). |
sbt-plugin/src/sbt-test/nomad/clean-and-migrate-postgres/src/main/scala/Main.scala |
Adds Postgres tests for missing-schema self-heal, opt-out reproducing original bug symptom, and canonical public-schema repro + idempotency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…res instances Test 9 was started with a bare EmbeddedPostgres.start(), bypassing the NOMAD_PG_TARBALL resolver applied to the first instance. On NixOS, the fallback to zonky's bundled generic-Linux binaries failed with the dynamic-linker error from stub-ld. Extract the bootstrap into startEmbeddedPostgres() and route both instances through it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
NovaMage
reviewed
May 11, 2026
…path Postgres checks ACL_CREATE on the database before IF NOT EXISTS short- circuits in CREATE SCHEMA, so the previous unconditional CREATE SCHEMA IF NOT EXISTS regressed least-privilege roles holding USAGE on a pre- existing schema but lacking CREATE on the database — for these callers, the 0.1.0 path that simply migrated against an existing schema would fail under 0.1.1 with a permission error even when no creation is actually needed. Probe via information_schema.schemata (readable by PUBLIC, no privilege required) and only issue CREATE SCHEMA on the missing-schema path. The no-op case for an already-existing schema now costs a single SELECT against a system view and demands no extra privilege, restoring the 0.1.0 privilege model for the common case while keeping the self-heal behavior intact when the schema is genuinely absent. Add Test 10 in clean-and-migrate-postgres exercising the regression directly: a freshly created role with USAGE+CREATE on a pre-existing schema and no CREATE on the database successfully runs migrate() with the default autoCreateSchema=true. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
NovaMage
reviewed
May 11, 2026
…OT EXISTS The probe-then-create sequence is TOCTOU-racy on cold start of a fresh database from horizontally scaled apps: two processes can both observe the schema as missing via the probe, then race the CREATE, and the loser fails with 'schema already exists'. Restore IF NOT EXISTS on the create branch only. The probe still filters out the existing-schema case, so least-privilege roles with USAGE on a pre-existing schema remain unaffected — the ACL_CREATE check IF NOT EXISTS triggers is only reached when the schema was actually missing at probe time, where forward progress already demands CREATE on the database. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrator.cleanAndMigrate()andMigrator.migrate()no longer fail unrecoverably when the configured schema does not exist (no schema has been selected to create in).isSchemaEmptyconflated "empty" with "missing", socleanAndMigrateskipped the recreate branch andmigrate()then hit the unqualifiedCREATE TYPE nomad_migration_type. The failure was self-perpetuating.autoCreateSchema: Boolean = trueconstructor parameter drivesCREATE SCHEMA IF NOT EXISTSat the start of both entry points. Opt-out preserved for least-privilege roles.0.1.0preserved via a secondary 5-argMigratorconstructor; reflection in the plugin selects the primary constructor by maximum arity so it stays deterministic.nomadSchema(default"public") andnomadAutoCreateSchema(defaulttrue) settings — the schema is no longer hard-coded inside the plugin.Test plan
sbt scripted nomad/clean-and-migrate(H2) — self-heal + opt-out assertionssbt scripted nomad/clean-and-migrate-postgres— Tests 1-9 including canonicalDROP SCHEMA public CASCADErepro + idempotent second callsbt scripted nomad/migrate-custom-schema—migrate()entry point, self-heal + opt-outsbt scripted nomad/migrate— plugin task still works with refactored settingsnomadMigrate, confirmed the schema was recreated and migrations applied.Test coverage notes
autoCreateSchema=false— pins the regression.DROP SCHEMA public CASCADE→cleanAndMigrate) plus a second invocation proving the fix is idempotent against the self-perpetuating nature of the original failure.SQLExceptionthat references the missing schema name — catches any future regression where the opt-out accidentally starts auto-creating.