Skip to content

feat(storage): add PostgreSQL EnsureSchema bootstrapper - #946

Merged
Kiran01bm merged 2 commits into
mainfrom
kiran01bm/postgres-ensure-schema
Aug 6, 2026
Merged

feat(storage): add PostgreSQL EnsureSchema bootstrapper#946
Kiran01bm merged 2 commits into
mainfrom
kiran01bm/postgres-ensure-schema

Conversation

@Kiran01bm

@Kiran01bm Kiran01bm commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds the PostgreSQL EnsureSchema bootstrapper — the Postgres counterpart to the MySQL schema bootstrap. EnsureSchema now routes the postgres storage dialect to a Postgres-specific bootstrap path instead of failing closed.

What

Why

A Postgres-backed state store must be able to bootstrap its own schema the same way the MySQL store does.

Deliberately no column/index drift repair on existing Postgres tables: the MySQL bootstrapper's drift repair rides on Spirit's diff/apply, which is MySQL-only. Schema evolution on an already-bootstrapped Postgres store is tracked separately under PLAT-38417.

┌───────────────────────┐   all tables exist    ┌──────┐
│ fast-path existence   │──────────────────────▶│ done │
│ check (no lock)       │                       └──────┘
└──────────┬────────────┘                           ▲
           │ missing tables                         │
           ▼                                        │
┌───────────────────────┐    ┌────────────────┐    │
│ acquire schema        │───▶│ re-check under │────┤ all exist
│ advisory lock (#938)  │    │ lock           │    │
└───────────────────────┘    └───────┬────────┘    │
                                     │ still missing
                                     ▼             │
                             ┌────────────────┐    │
                             │ create missing │────┘
                             │ tables (one tx │
                             │ per table)     │
                             └────────────────┘

References

Postgres counterpart to the MySQL schema bootstrap: existence-only
convergence that creates missing storage tables from the embedded
Postgres schema files, serialized across pods by the advisory-lock
locker with a fast-path existence check re-verified under the lock.
Deliberately no column/index drift repair — Spirit's diffing is
MySQL-only, so schema evolution on an already-bootstrapped Postgres
store lands separately (PLAT-38417).
Copilot AI lite review requested due to automatic review settings August 6, 2026 03:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds PostgreSQL support to SchemaBot’s startup storage-schema bootstrap by introducing a Postgres-specific EnsureSchema path (existence-only table creation) and routing schema.DialectPostgres to it, bringing Postgres storage bootstrapping in line with the existing MySQL experience.

Changes:

  • Route EnsureSchema to a new PostgreSQL bootstrapper when WithDialect(schema.DialectPostgres) is selected.
  • Implement ensurePostgresSchema to create missing storage tables from embedded Postgres DDL under a cross-pod advisory lock (re-checking under lock; one transaction per table).
  • Add unit + integration coverage for routing, statement splitting, embedded schema reading, idempotence, missing-table repair, and concurrent pod startup behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pkg/api/ensure_schema.go Adds Postgres dialect routing and updates the fail-closed error to list supported dialects.
pkg/api/ensure_schema_test.go Updates fail-closed test to use a truly unsupported dialect and adds a routing test for Postgres.
pkg/api/ensure_schema_postgres.go New Postgres bootstrapper: reads embedded schema files, checks for missing tables, serializes via advisory lock, and creates tables transactionally.
pkg/api/ensure_schema_postgres_test.go Unit tests for Postgres schema file splitting and embedded schema file reading invariants.
pkg/api/ensure_schema_postgres_integration_test.go Integration tests validating fresh bootstrap, idempotence, repair of missing tables, and safe concurrent bootstraps using a Postgres container.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/api/ensure_schema_postgres.go Outdated
@Kiran01bm Kiran01bm changed the title feat(schema): add PostgreSQL EnsureSchema bootstrapper feat(storage): add PostgreSQL EnsureSchema bootstrapper Aug 6, 2026
…apper

Delete the hand-rolled statement splitter (pgx's simple query protocol
executes multi-statement zero-argument Execs natively), pin the lock
contention deterministically via pg_locks, extract the shared Postgres
testcontainer starter, and align the flow's logging, timeout docs, and
operator docs with the MySQL bootstrapper.
@Kiran01bm
Kiran01bm marked this pull request as ready for review August 6, 2026 07:27
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@aparajon

aparajon commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

🤖 Adversarial correctness review, requested by Armand and performed by his agent. Reviewed at head 5ca0874.

Verdict: clean — no fix-before-merge findings. The existence-only scope matches the settled direction for this slice (create-if-absent bring-up now; declarative diff/evolution lands separately with its own mechanism), the concurrency story is right, and the pieces I attacked hardest all held.

One design note, non-blocking: existence-only convergence means a table that exists with the wrong shape is accepted silently at startup and surfaces later as runtime query errors (e.g. a binary rollback after a newer binary bootstrapped a newer-shape table, or the documented new-column-on-existing-table gap). That's the deliberate, documented bound — and detecting drift properly is exactly the deferred diff mechanism, so building it here would be scope creep. If a cheap startup-time tripwire ever becomes worth it before the real mechanism lands, the new plain-statements lint invariant makes a column-name presence check against information_schema.columns feasible without a SQL parser — a fail-at-startup instead of fail-at-first-query. Fine to leave to the separately tracked evolution work.

Verified solid:

  • The multi-statement Exec claim is real, not hopeful. createPostgresTable runs each whole file in one zero-argument ExecContext; pgx's stdlib driver routes argument-less Execs through the simple query protocol, which executes multi-statement strings natively — and the integration tests prove it against postgres:16 (extended-protocol prepared statements would have rejected the multi-statement files outright). The per-file transaction gives the table-with-all-its-indexes-or-nothing invariant the comment promises, and the new TestPostgresFilesContainOnlyPlainStatements lint pins the file shape this depends on.
  • The advisory-lock helper honors the namedlock caller contract. acquirePostgresEnsureSchemaLock opens a dedicated *sql.DB and closes the pool before returning, so closing the returned connection genuinely terminates the session and releases the lock — the comment spells out why a shared-pool return would not. This is the safe call-site pattern for namedlock.Postgres, and the locker's own acquire-error paths (undoAcquire on commit failure and on a cancellation racing the grant, WithoutCancel + discard) cover the rest.
  • Fast path → lock → re-check is TOCTOU-sound. The unlocked fast path is read-only; a pod that finds tables missing serializes on the lock and re-verifies before creating, and the per-table transactionality means a fast-path reader can never observe a table without its indexes. TestEnsureSchemaPostgres_WaitsForAdvisoryLock pins the contention deterministically via pg_locks (ungranted advisory waiter) rather than sleeping, and proves nothing is created while parked.
  • Fail-closed edges: empty embedded directory errors; a search_path that resolves to no schema makes the diagnostic coalesce but the existence listing then reports everything missing and CREATE TABLE surfaces the server's real error; the dual-deadline contended-timeout case (client ctx expires before the server-side lock wait) is named in the error on both the new PG helper and, nicely, backported to the MySQL helper; the dialect dispatch still fails closed for unknown dialects with the routing test updated to a genuinely unsupported dialect.
  • The docs/configuration.md and AGENTS.md updates state the PG bound accurately, including that allow_destructive_schema_changes has no effect on a flow that can only create. The testutil.StartPostgres extraction dedupes the container boilerplate, and PostgresTableExists correctly uses $1 binds where the MySQL helper's ? would not bind.

Build, unit (-race), and all five PG bootstrap integration tests plus the schema-parity suite green locally; CI green 32/32.

This review was generated by Claude Code (claude-fable-5).

@aparajon aparajon left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Approving per the adversarial review above — clean, no blocking findings. Stamped by Armand's agent (claude-fable-5).

@Kiran01bm
Kiran01bm merged commit 9098f5e into main Aug 6, 2026
32 checks passed
@Kiran01bm
Kiran01bm deleted the kiran01bm/postgres-ensure-schema branch August 6, 2026 23:25
Kiran01bm added a commit that referenced this pull request Aug 7, 2026
…urable-unlock-dispatch

* origin/main:
  fix(github): sanitize engine error text rendered into PR comments (#891)
  refactor(storage): depend on storage.Storage instead of *mysqlstore.Storage (#947)
  feat(storage): add PostgreSQL EnsureSchema bootstrapper (#946)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants