From 61a1c17746030caea19bece583a870cb8c8699fa Mon Sep 17 00:00:00 2001 From: Adrian Ehrsam Date: Tue, 1 Sep 2026 16:18:16 +0200 Subject: [PATCH 1/3] Fix migrate apply --ask output interleaving with the background worker queue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The worker thread applying queued migrations and the --ask review loop both write to the tqdm bar, but only individual write calls were locked (2cbacb5) — the worker could still print an "Applied ..." status line mid-render of the next file's SQL or the confirm prompt itself. Hold bar_lock across the whole show-file+prompt step so worker output defers until the question is answered, instead of interleaving with it. Bump version to 0.3.5. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01SqXjRpDFprwfALdveFDewa --- pgdevkit/cli.py | 14 +++++++++++--- pyproject.toml | 2 +- uv.lock | 2 +- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/pgdevkit/cli.py b/pgdevkit/cli.py index 64ca5a1..e421a48 100644 --- a/pgdevkit/cli.py +++ b/pgdevkit/cli.py @@ -371,9 +371,17 @@ def worker() -> None: break already_done = False if ask: - bar_write(f"\n=== {path.name} ===") - bar_write(path.read_text(encoding="utf-8")) - answer = typer.prompt("[Y]es execute / [n]o skip / [a]lready done / [q]uit", default="y").strip().lower() + # Hold the lock across the whole show-file+prompt step (not just each write) so the + # worker's bar_write() calls block until the question is answered instead of + # interleaving with the prompt or the migration text while the user is reading it. + with bar_lock: + bar.write(f"\n=== {path.name} ===") + bar.write(path.read_text(encoding="utf-8")) + answer = ( + typer.prompt("[Y]es execute / [n]o skip / [a]lready done / [q]uit", default="y") + .strip() + .lower() + ) if answer in ("q", "quit"): quit_requested = True break diff --git a/pyproject.toml b/pyproject.toml index 74b126a..9f10b4b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -11,7 +11,7 @@ packages = ["pgdevkit"] [project] name = "pgdevkit" -version = "0.3.4" +version = "0.3.5" description = "A helper for developing with Postgres" readme = "README.md" requires-python = ">=3.14" diff --git a/uv.lock b/uv.lock index 37034a2..0c25038 100644 --- a/uv.lock +++ b/uv.lock @@ -313,7 +313,7 @@ wheels = [ [[package]] name = "pgdevkit" -version = "0.3.4" +version = "0.3.5" source = { editable = "." } dependencies = [ { name = "docker" }, From 835bc8480dccc15f99bba55de96eafa99335a4db Mon Sep 17 00:00:00 2001 From: Adrian Ehrsam Date: Tue, 1 Sep 2026 17:40:37 +0200 Subject: [PATCH 2/3] Auto-answer "already done" in migrate apply --ask for trivially no-op migrations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Detect when every statement in a migration is a create-if-missing shape (CREATE TABLE/INDEX/SEQUENCE/VIEW/SCHEMA IF NOT EXISTS, or ALTER TABLE ADD COLUMN) whose target already exists in the database, and skip the prompt entirely in that case — record it as already done instead of asking. CREATE OR REPLACE and any other statement shape are never auto-detected, since existence alone doesn't mean re-running would be a no-op. Bump version to 0.3.6. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01SqXjRpDFprwfALdveFDewa --- pgdevkit/cli.py | 52 +++++++++++++++-------------- pgdevkit/migrate.py | 77 +++++++++++++++++++++++++++++++++++++++++++ pyproject.toml | 2 +- tests/test_migrate.py | 60 ++++++++++++++++++++++++++++++++- uv.lock | 2 +- 5 files changed, 166 insertions(+), 27 deletions(-) diff --git a/pgdevkit/cli.py b/pgdevkit/cli.py index e421a48..b89b18a 100644 --- a/pgdevkit/cli.py +++ b/pgdevkit/cli.py @@ -371,30 +371,34 @@ def worker() -> None: break already_done = False if ask: - # Hold the lock across the whole show-file+prompt step (not just each write) so the - # worker's bar_write() calls block until the question is answered instead of - # interleaving with the prompt or the migration text while the user is reading it. - with bar_lock: - bar.write(f"\n=== {path.name} ===") - bar.write(path.read_text(encoding="utf-8")) - answer = ( - typer.prompt("[Y]es execute / [n]o skip / [a]lready done / [q]uit", default="y") - .strip() - .lower() - ) - if answer in ("q", "quit"): - quit_requested = True - break - if answer in ("n", "no"): - bar_write(f"Skipped {path.name}") - outcomes.append((path.name, "skipped")) - bar_step() - continue - if answer in ("a", "already", "already done"): - already_done = True - elif answer not in ("", "y", "yes"): - bar_write(f"Skipped {path.name}") - outcomes.append((path.name, "skipped")) + already_done = migrate.already_fully_applied(conninfo, path) + if already_done: + bar_write(f"Auto: {path.name} is already fully present in the database — marking as already done") + else: + # Hold the lock across the whole show-file+prompt step (not just each write) so + # the worker's bar_write() calls block until the question is answered instead + # of interleaving with the prompt or the migration text while it's being read. + with bar_lock: + bar.write(f"\n=== {path.name} ===") + bar.write(path.read_text(encoding="utf-8")) + answer = ( + typer.prompt("[Y]es execute / [n]o skip / [a]lready done / [q]uit", default="y") + .strip() + .lower() + ) + if answer in ("q", "quit"): + quit_requested = True + break + if answer in ("n", "no"): + bar_write(f"Skipped {path.name}") + outcomes.append((path.name, "skipped")) + bar_step() + continue + if answer in ("a", "already", "already done"): + already_done = True + elif answer not in ("", "y", "yes"): + bar_write(f"Skipped {path.name}") + outcomes.append((path.name, "skipped")) bar_step() continue diff --git a/pgdevkit/migrate.py b/pgdevkit/migrate.py index 8ffa181..5cd377d 100644 --- a/pgdevkit/migrate.py +++ b/pgdevkit/migrate.py @@ -197,6 +197,83 @@ def _created_table_names(stmts: list[str]) -> list[str]: return names +_ADD_COLUMN_RE = re.compile( + rf"ALTER\s+TABLE\s+(?:IF\s+EXISTS\s+)?([\w.\"]+)\s+ADD\s+COLUMN\s+" + rf"(?:IF\s+NOT\s+EXISTS\s+)?\"?({_IDENTIFIER})\"?", + re.IGNORECASE, +) +_CREATE_RELATION_RE = re.compile( + r"CREATE\s+(?:UNIQUE\s+)?INDEX\s+(?:CONCURRENTLY\s+)?(?:IF\s+NOT\s+EXISTS\s+)?([\w.\"]+)" + r"|CREATE\s+SEQUENCE\s+(?:IF\s+NOT\s+EXISTS\s+)?([\w.\"]+)" + r"|CREATE\s+(?:MATERIALIZED\s+)?VIEW\s+(?:IF\s+NOT\s+EXISTS\s+)?([\w.\"]+)", + re.IGNORECASE, +) +_CREATE_SCHEMA_RE = re.compile(r"CREATE\s+SCHEMA\s+(?:IF\s+NOT\s+EXISTS\s+)?([\w\"]+)", re.IGNORECASE) + + +def _idempotent_target(stmt: str) -> tuple[str, ...] | None: + """Classify a single statement as one of the create-if-missing DDL shapes this module + can check for "already applied" with no ambiguity: ("relation", name) for a table, + index, sequence or view; ("schema", name); or ("column", table, column) for an ADD + COLUMN. None if the statement isn't one of these shapes — including any CREATE OR + REPLACE, which is never safe to treat as a no-op just because the object exists, since + the migration could be replacing it with different content.""" + stripped = _strip_line_comments(stmt).strip() + if re.search(r"\bOR\s+REPLACE\b", stripped, re.IGNORECASE): + return None + + if re.match(r"CREATE\s+TABLE\b", stripped, re.IGNORECASE): + names = _created_table_names([stmt]) + return ("relation", names[0]) if names else None + + m = _CREATE_RELATION_RE.match(stripped) + if m: + name = next(g for g in m.groups() if g is not None) + return ("relation", name) + + m = _CREATE_SCHEMA_RE.match(stripped) + if m: + return ("schema", m.group(1)) + + m = _ADD_COLUMN_RE.match(stripped) + if m: + return ("column", m.group(1), m.group(2)) + + return None + + +def _target_exists(con: psycopg.Connection, target: tuple[str, ...]) -> bool: + kind = target[0] + if kind == "relation": + row = con.execute("select to_regclass(%s)", (target[1],)).fetchone() + elif kind == "schema": + row = con.execute("select to_regnamespace(%s)", (target[1],)).fetchone() + else: # column + row = con.execute( + "select 1 from pg_attribute where attrelid = to_regclass(%s) " + "and attname = %s and not attisdropped", + (target[1], target[2]), + ).fetchone() + return bool(row and row[0]) + + +def already_fully_applied(conninfo: str, path: Path) -> bool: + """Whether every statement in this migration is a recognized create-if-missing shape + (table/index/sequence/view/schema, or add-column) AND its target already exists in the + database — i.e. re-running the migration would do nothing. Used by `--ask` to + auto-answer "already done" without prompting, so migrations that are trivially no-ops + don't interrupt review. A single unrecognized or not-yet-applied statement means + False — this never guesses.""" + stmts = _split_sql(path.read_text(encoding="utf-8")) + if not stmts: + return False + targets = [_idempotent_target(s) for s in stmts] + if any(t is None for t in targets): + return False + with psycopg.connect(conninfo) as con: + return all(_target_exists(con, cast(tuple[str, ...], t)) for t in targets) + + def list_migration_files(migrations_dir: Path) -> list[Path]: return sorted(migrations_dir.glob("*.sql")) diff --git a/pyproject.toml b/pyproject.toml index 9f10b4b..f3ab061 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -11,7 +11,7 @@ packages = ["pgdevkit"] [project] name = "pgdevkit" -version = "0.3.5" +version = "0.3.6" description = "A helper for developing with Postgres" readme = "README.md" requires-python = ">=3.14" diff --git a/tests/test_migrate.py b/tests/test_migrate.py index d80a608..778c394 100644 --- a/tests/test_migrate.py +++ b/tests/test_migrate.py @@ -1,6 +1,12 @@ from __future__ import annotations -from pgdevkit.migrate import _created_table_names, _split_sql, _strip_line_comments, default_tracking_table +from pgdevkit.migrate import ( + _created_table_names, + _idempotent_target, + _split_sql, + _strip_line_comments, + default_tracking_table, +) def test_created_table_names_ignores_create_table_mentioned_in_a_comment(): @@ -74,3 +80,55 @@ def test_default_tracking_table_searches_parent_directories(tmp_path): def test_default_tracking_table_falls_back_with_no_pyproject_at_all(tmp_path): assert default_tracking_table(tmp_path / "nonexistent") == "public.schema_migrations" + + +def test_idempotent_target_recognizes_create_table_if_not_exists(): + assert _idempotent_target("CREATE TABLE IF NOT EXISTS app.widgets (id int)") == ("relation", "app.widgets") + + +def test_idempotent_target_recognizes_create_index(): + assert _idempotent_target( + "CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS widgets_name_idx ON app.widgets (name)" + ) == ("relation", "widgets_name_idx") + + +def test_idempotent_target_recognizes_create_sequence(): + assert _idempotent_target("CREATE SEQUENCE IF NOT EXISTS app.widgets_seq") == ("relation", "app.widgets_seq") + + +def test_idempotent_target_recognizes_create_view(): + assert _idempotent_target("CREATE VIEW IF NOT EXISTS app.widgets_v AS SELECT * FROM app.widgets") == ( + "relation", + "app.widgets_v", + ) + + +def test_idempotent_target_recognizes_create_schema(): + assert _idempotent_target("CREATE SCHEMA IF NOT EXISTS app") == ("schema", "app") + + +def test_idempotent_target_recognizes_add_column(): + assert _idempotent_target("ALTER TABLE app.widgets ADD COLUMN IF NOT EXISTS name text") == ( + "column", + "app.widgets", + "name", + ) + + +def test_idempotent_target_recognizes_add_column_without_if_not_exists_guard(): + assert _idempotent_target("ALTER TABLE app.widgets ADD COLUMN name text") == ( + "column", + "app.widgets", + "name", + ) + + +def test_idempotent_target_ignores_create_or_replace(): + # A view/function could be replaced with different content, so existence alone never + # means "already applied" for these. + assert _idempotent_target("CREATE OR REPLACE VIEW app.widgets_v AS SELECT 1") is None + + +def test_idempotent_target_ignores_unrecognized_statements(): + assert _idempotent_target("ALTER TABLE app.widgets DROP COLUMN name") is None + assert _idempotent_target("INSERT INTO app.widgets (id) VALUES (1)") is None diff --git a/uv.lock b/uv.lock index 0c25038..226d0fd 100644 --- a/uv.lock +++ b/uv.lock @@ -313,7 +313,7 @@ wheels = [ [[package]] name = "pgdevkit" -version = "0.3.5" +version = "0.3.6" source = { editable = "." } dependencies = [ { name = "docker" }, From a5a21e288da91a19a749d648a851e36cca83ca0b Mon Sep 17 00:00:00 2001 From: Adrian Ehrsam Date: Tue, 1 Sep 2026 17:50:54 +0200 Subject: [PATCH 3/3] Fix migrate apply --ask silently skipping every accepted/already-done migration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The trailing bar_step()/continue at the end of the answer if/elif chain weren't re-indented when the auto-detect branch's else: added a nesting level, so they fired unconditionally after the chain instead of only on the unrecognized-answer path — meaning answering "y" (default accept) or "a" (already done) fell through to bar_step()/continue before reaching work_q.put(), so the migration was never enqueued, applied, or recorded. Caught by /code-review. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01SqXjRpDFprwfALdveFDewa --- pgdevkit/cli.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pgdevkit/cli.py b/pgdevkit/cli.py index b89b18a..f8968b7 100644 --- a/pgdevkit/cli.py +++ b/pgdevkit/cli.py @@ -399,8 +399,8 @@ def worker() -> None: elif answer not in ("", "y", "yes"): bar_write(f"Skipped {path.name}") outcomes.append((path.name, "skipped")) - bar_step() - continue + bar_step() + continue work_q.put((path, already_done))