Mdmapp migration

pgdevkit/testdb/schema.py

@@ -172,54 +172,19 @@ async def _insert_test_data(
         await cur.executemany(insert_sql, rows)
 
 
-_UNSAFE_MIGRATION_PATTERNS = (
-    "DROP COLUMN",
-    "DROP TABLE",
-    "TRUNCATE",
-    "RENAME COLUMN",
-    "RENAME TO",
-    "DELETE FROM",
-    "OWNER TO",
-)
-
-
-def _is_additive_migration(sql: str) -> bool:
-    """Only pure-additive migrations (ADD COLUMN/CREATE ... IF NOT EXISTS,
-    etc.) are safe to (re-)apply against a schema that already reflects
-    later migrations — skip anything that could drop, rename, truncate, or
-    delete existing structure/data, alter an existing column, or change
-    ownership. This is a substring heuristic, not a SQL parser, so it errs
-    towards skipping (a false negative here means real data loss replayed
-    on every apply_schema() call) rather than trying to be exhaustive."""
-    normalized = sql.upper()
-    has_alter_column = "ALTER COLUMN" in normalized and "ADD COLUMN" not in normalized
-    has_unsafe_pattern = any(pattern in normalized for pattern in _UNSAFE_MIGRATION_PATTERNS)
-    return not (has_alter_column or has_unsafe_pattern)
-
-
-def _iter_migration_files(database_dir: Path):
-    """Yield every `*.sql` file in any `migrations/` subdirectory, in
-    filename order (the project's naming convention — numeric or date
-    prefixes — sorts chronologically)."""
-    for root, _, files in os.walk(database_dir):
-        if Path(root).name != "migrations":
-            continue
-        for file in sorted(files):
-            if file.endswith(".sql"):
-                yield Path(root) / file
-
-
 async def apply_schema(
     con: psycopg.AsyncConnection,
     database_dir: Path,
     extensions: tuple[str, ...] = (),
     force_reset: bool = False,
 ) -> None:
-    """Apply every .sql file under database_dir (in dependency-safe order),
-    seed any matching .test_data.json files, then apply purely-additive
-    `migrations/*.sql` files (see `_is_additive_migration`) so a schema that
-    ships changes via migration files rather than editing the base object
-    files stays in sync. Safe to call repeatedly."""
+    """Apply every .sql file under database_dir (in dependency-safe order)
+    and seed any matching .test_data.json files. Safe to call repeatedly.
+
+    `migrations/` subdirectories are never applied here — they're for
+    one-time manual application against real (already-provisioned)
+    databases, not for building a fresh schema. The base object files under
+    `database_dir` must reflect the current, final schema on their own."""
     await con.set_autocommit(True)
     for extension in extensions:
         await con.execute(SQL("CREATE EXTENSION IF NOT EXISTS {e}").format(e=Identifier(extension)))
@@ -243,13 +208,3 @@ async def _apply(file: Path, sql: str) -> None:
 
     for file, sql in failures:
         await _apply(file, sql)
-
-    for migration_file in _iter_migration_files(database_dir):
-        content = migration_file.read_text(encoding="utf-8")
-        if not _is_additive_migration(content):
-            continue
-        try:
-            await con.execute(cast(Any, content))
-            logger.info("Applied migration %s", migration_file.name)
-        except Exception as e:  # noqa: BLE001
-            logger.debug("Migration %s skipped (likely already applied): %s", migration_file.name, e)

pyproject.toml

@@ -11,7 +11,7 @@ packages = ["pgdevkit"]
 
 [project]
 name = "pgdevkit"
-version = "0.2.0"
+version = "0.2.1"
 description = "A helper for developing with Postgres"
 readme = "README.md"
 requires-python = ">=3.14"

skills/pgdevkit/SKILL.md

@@ -75,6 +75,8 @@ def _testdb_env():
 
 `ensure_testdb()` starts the shared `pgdevkit-postgres` container if needed (via `podman`), creates a database scoped to this project+branch (so different worktrees/branches never collide), and applies every `.sql` file under `database_dir` in dependency order, seeding any `.test_data.json` sidecar files.
 
+**`migrations/` is never applied here, on purpose.** If the test schema is missing something, that's a sign the base `tables/`/`views`/... file has drifted behind a migration that was only ever run manually against a real database — fix the base file, don't add migration-replay to `apply_schema()` (tried once, reverted: a migration can't be judged "safe to re-run" from its SQL text alone — see `docs/database-layout.md`'s Migrations section).
+
 | What do you need? | Command |
 |---|---|
 | First-time setup / apply new files | `pgdb testdb up` |

tests/testdb/test_schema.py

@@ -7,34 +7,13 @@
 
 from pgdevkit.testdb import constants
 from pgdevkit.testdb.container import ensure_container
-from pgdevkit.testdb.schema import _is_additive_migration, apply_schema
+from pgdevkit.testdb.schema import apply_schema
 from tests.testdb.conftest import RUN_SUFFIX, requires_podman
 
 FIXTURES = Path(__file__).parent / "fixtures" / "database"
 TEST_DB = f"pgdevkit_schema_selftest_{RUN_SUFFIX}"
 
 
-@pytest.mark.parametrize(
-    "sql,expected",
-    [
-        ("ALTER TABLE t ADD COLUMN IF NOT EXISTS x text;", True),
-        ("CREATE TABLE IF NOT EXISTS t (id serial primary key);", True),
-        ("CREATE INDEX IF NOT EXISTS idx ON t(x);", True),
-        ("ALTER TABLE t ALTER COLUMN x TYPE text;", False),
-        ("ALTER TABLE t DROP COLUMN x;", False),
-        ("DROP TABLE t;", False),
-        ("DROP TABLE IF EXISTS t;", False),
-        ("TRUNCATE t;", False),
-        ("ALTER TABLE t RENAME COLUMN x TO y;", False),
-        ("ALTER TABLE t RENAME TO t2;", False),
-        ("DELETE FROM t WHERE id = 1;", False),
-        ("ALTER TABLE t OWNER TO someone;", False),
-    ],
-)
-def test_is_additive_migration(sql, expected):
-    assert _is_additive_migration(sql) is expected
-
-
 def _admin_dsn() -> str:
     return constants.conninfo("postgres")
 
@@ -136,25 +115,11 @@ async def test_apply_schema_seeds_composite_enum_and_jsonb_columns(schema_test_d
 
 
 @requires_podman
-async def test_apply_schema_applies_additive_migrations(schema_test_db):
-    # app/migrations/001_add_gadget_note.sql adds a column not present in
-    # gadget.sql itself — only reachable via the migrations pass.
-    async with await psycopg.AsyncConnection.connect(_db_dsn(), autocommit=True) as con:
-        await apply_schema(con, FIXTURES)
-        async with con.cursor() as cur:
-            await cur.execute(
-                "SELECT column_name FROM information_schema.columns "
-                "WHERE table_schema = 'app' AND table_name = 'gadget' AND column_name = 'note'"
-            )
-            row = await cur.fetchone()
-    assert row is not None
-
-
-@requires_podman
-async def test_apply_schema_skips_unsafe_migrations(schema_test_db):
-    # app/migrations/002_unsafe_drop_gadget_note.sql drops the column that
-    # 001 added — apply_schema must skip it (not execute it), so the column
-    # added by 001 must still be present afterwards.
+async def test_apply_schema_never_applies_migrations_dir(schema_test_db):
+    # migrations/ is for one-time manual application against real databases,
+    # not for building a fresh schema — the base table file is the only
+    # source of truth apply_schema uses. app/migrations/001_add_gadget_note.sql
+    # adds a column that gadget.sql itself doesn't have; it must NOT appear.
     async with await psycopg.AsyncConnection.connect(_db_dsn(), autocommit=True) as con:
         await apply_schema(con, FIXTURES)
         async with con.cursor() as cur:
@@ -163,4 +128,4 @@ async def test_apply_schema_skips_unsafe_migrations(schema_test_db):
                 "WHERE table_schema = 'app' AND table_name = 'gadget' AND column_name = 'note'"
             )
             row = await cur.fetchone()
-    assert row is not None
+    assert row is None