Skip to content

feat(pgsql-test): deferredConstraints — run commit-time deferred constraint checks under rollback isolation - #1859

Merged
pyramation merged 1 commit into
mainfrom
devin/1790168515-pgsql-test-deferred-constraints
Sep 23, 2026
Merged

pyramation merged 1 commit into
mainfrom
devin/1790168515-pgsql-test-deferred-constraints

Conversation

@pyramation

Copy link
Copy Markdown
Contributor

Summary

Rollback-based isolation (BEGIN; SAVEPOINT … ROLLBACK TO SAVEPOINT; COMMIT) never commits, so DEFERRABLE INITIALLY DEFERRED constraints (FK / UNIQUE / EXCLUDE / CONSTRAINT TRIGGER) — which Postgres only checks at COMMIT — never fire. A test can leave a dangling deferred FK and pass. This is inherent to every rollback harness (Rails, Django, Ecto…); this PR adds an opt-in way to close the hole without changing semantics inside the test.

New option db.deferredConstraints: 'off' | 'check' | 'immediate' (PgTestConnectionOptions, env DB_DEFERRED_CONSTRAINTS), flowing through getConnections to both pg and db clients — and therefore to graphile-test / graphql-test / graphql-server-test for free.

  • 'off' (default) — unchanged behaviour. Fully backwards compatible.
  • 'check' — afterEach() issues SET CONSTRAINTS ALL IMMEDIATE as the last statement before the rollback. Per the Postgres docs, switching DEFERRED→IMMEDIATE runs exactly the checks COMMIT would have run on the outstanding modifications, so deferral still works inside the test body but a pending violation fails the test. Caveats handled:
    • if the test body already left the tx aborted (25P02), the check is skipped so the original error isn't masked;
    • rollback + commit always run; the violation is rethrown afterwards so the client is reusable by the next test;
    • error is prefixed [pgsql-test] deferred constraint violated at end of test (a real COMMIT would have failed here) and keeps the PG detail/constraint name.
  • 'immediate' — beforeEach() (and publish()) issue SET CONSTRAINTS ALL IMMEDIATE, so violations fail on the offending statement. Documented trade-off: disables deferral the code under test may rely on (child-before-parent, unique swaps).

New public db.checkConstraints() lets a test assert enforcement explicitly (await expect(db.checkConstraints()).rejects.toThrow(...)); once it throws the pending events are consumed and afterEach is clean.

async afterEach() {
  let violation;
  if (mode === 'check') try { await this.checkConstraints(); } catch (e) { if (e.code !== '25P02') violation = e; }
  await this.rollback(); await this.commit();
  if (violation) throw violation;
}

README gains a "Deferred constraints under rollback isolation" section covering the three modes, their trade-offs, and what rollback isolation still does not cover (NOTIFY, cross-session visibility → publish()).

Tests (postgres-test.deferred-constraints.test.ts, 12 cases) cover all three modes incl. aborted-tx path, publish() re-application, checkConstraints() consumption, and the default being 'off'.

Follow-up decision: whether to flip the default to 'check' in a minor release.

Link to Devin session: https://app.devin.ai/sessions/b842729ae6934f2093de8a43018c36fb
Open in Devin Desktop: https://app.devin.ai/desktop/session/b842729ae6934f2093de8a43018c36fb?variant=devin
Requested by: @pyramation

…ed constraint checks under rollback isolation
@devin-ai-integration

Copy link
Copy Markdown
Contributor

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@tenki-reviewer

tenki-reviewer Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Review complete. No blocking issues — approved ✅; 1 nitpick below.

🧹 Nitpicks (1) — 🟢 1 low
  • 🟢 Throw EnvError for DB_DEFERRED_CONSTRAINTS parsing (env.ts:11) — parseDeferredConstraintsMode throws a plain Error for an invalid DB_DEFERRED_CONSTRAINTS value (pgpm/env/src/env.ts:11).

This PR introduces an opt-in deferred-constraints checking mode for the pgsql-test harness: a new deferredConstraints option (off default, immediate, check) threaded from DB_DEFERRED_CONSTRAINTS env parsing through getConnections into both pg and db test clients, with checkConstraints applied at transaction end and a new test suite covering the modes.

Files Change
pgpm/env/src/env.ts, pgpm/types/src/pgpm.ts Parse DB_DEFERRED_CONSTRAINTS into a typed mode enum and extend the shared options type.
postgres/pgsql-test/src/connect.ts Propagate the new option into pg and db client construction.
postgres/pgsql-test/src/test-client.ts Apply SET CONSTRAINTS ALL IMMEDIATE per transaction end and validate violations in check mode.
postgres/pgsql-test/__tests__/postgres-test.deferred-constraints.test.ts New tests covering default-off, immediate, and check behavior.
postgres/pgsql-test/README.md Document the new option.

Reviewed commit: 2ebafde

@pyramation
pyramation merged commit 70eb5e4 into main Sep 23, 2026
21 checks passed
@pyramation
pyramation deleted the devin/1790168515-pgsql-test-deferred-constraints branch September 23, 2026 23:31
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.

1 participant