fix: re-emit VALIDATE for an unvalidated PG18 NOT NULL constraint (#564)

[tianzhou]

Summary

Follow-up to #566, which added the PG18 native ADD CONSTRAINT ... NOT NULL ... NOT VALID + VALIDATE CONSTRAINT rewrite. That PR noted a gap: if apply is interrupted after the ADD but before the VALIDATE (or someone adds a NOT VALID NOT NULL constraint by hand), PostgreSQL already sets attnotnull, so the inspector saw the column as NOT NULL and the next plan was empty. Existing rows were never validated and there was no way to notice.

This is the "remind on next run" part of #564.

Fix

• GetColumns/GetColumnsForSchema now join pg_constraint for a contype = 'n' row with convalidated = false on the column and expose it as invalid_not_null_constraint (empty on PG < 18, where that contype does not exist). sqlc regenerated.
• New Column.InvalidNotNullConstraint IR field, only ever populated on the current state.
• columnsEqual treats a NOT NULL desired column with a pending constraint as modified, and generateColumnSQL emits ALTER TABLE ... VALIDATE CONSTRAINT <name>; for it.
• The plan layer isolates that step in its own transaction, like every other VALIDATE, so the scan does not share a transaction with lock-taking DDL.
• Dump output is unchanged: the column still dumps as NOT NULL, which is the desired end state, and plan shows the pending VALIDATE.

Out of scope (by design, see #564 discussion)

• Auto-splitting a brand-new ADD COLUMN ... NOT NULL: without a backfill VALIDATE would still fail, and pgschema cannot infer the backfill. The manual workflow is documented in Custom Migration Steps.
• Event-trigger driven backfills: pgschema does not manage event triggers.

Closes #564

Test plan

• Folded the scenario into testdata/diff/online/add_not_null: old.sql adds phone text plus ADD CONSTRAINT users_phone_not_null NOT NULL phone NOT VALID; new.sql declares phone text NOT NULL. Expected diff now includes VALIDATE CONSTRAINT users_phone_not_null, and plan fixtures were regenerated against embedded PG18 (apply + convergence verified).
• New unit test TestPendingNotNullValidateIsolated in internal/plan/rewrite_test.go checks the VALIDATE step lands in its own execution group.
• Ran locally: PGSCHEMA_TEST_FILTER="online/" go test ./internal/diff, go test ./internal/plan, PGSCHEMA_TEST_FILTER="online/add_not_null" go test ./cmd -run TestPlanAndApply, create_table/remove_not_null, and TestDumpCommand_Employee; all pass. Full suite left to CI.

🤖 Generated with Claude Code

[Copilot]

🟢 Approval recommended

The change set is coherent end-to-end (introspection → IR → diff emission → plan isolation) and includes both fixture coverage and a targeted unit test for the critical isolation behavior.

Pull request overview

This PR fixes a PostgreSQL 18+ edge case where a NOT NULL ... NOT VALID constraint can remain unvalidated (e.g., interrupted apply), but the column still appears NOT NULL via attnotnull, causing subsequent plans to incorrectly converge without re-emitting the required VALIDATE CONSTRAINT.

Changes:

• Extend column introspection to detect pending (unvalidated) PG18 contype = 'n' NOT NULL constraints and surface the constraint name in IR.
• Treat “desired NOT NULL + current has pending NOT NULL validation” as drift so diffs re-emit VALIDATE CONSTRAINT <name>.
• Ensure the emitted VALIDATE CONSTRAINT runs in its own execution group (transaction isolation), and update fixtures + add a plan-layer unit test.

File summaries

File	Description
testdata/diff/online/add_not_null/old.sql	Adds the repro scenario: a NOT NULL ... NOT VALID constraint left unvalidated.
testdata/diff/online/add_not_null/new.sql	Declares the column as NOT NULL in desired state to force convergence behavior.
testdata/diff/online/add_not_null/diff.sql	Expects re-emission of VALIDATE CONSTRAINT users_phone_not_null.
testdata/diff/online/add_not_null/plan.sql	Includes the extra VALIDATE CONSTRAINT step in the plan output.
testdata/diff/online/add_not_null/plan.txt	Reflects the additional column diff and isolated transaction group.
testdata/diff/online/add_not_null/plan.json	Adds a new execution group for the pending validation step.
ir/queries/queries.sql	Joins pg_constraint to detect unvalidated PG18 NOT NULL constraints per column.
ir/queries/queries.sql.go	Regenerated sqlc output to carry the new invalid_not_null_constraint field.
ir/ir.go	Adds Column.InvalidNotNullConstraint to represent pending PG18 validation state (current-state signal).
ir/inspector.go	Populates InvalidNotNullConstraint from query results during inspection.
internal/diff/column.go	Emits VALIDATE CONSTRAINT when both sides are NOT NULL but current has pending validation; also marks columns unequal in that case.
internal/plan/rewrite.go	Forces the pending VALIDATE CONSTRAINT step into its own execution group (isolation).
internal/plan/rewrite_test.go	Adds a unit test ensuring the pending validation step is isolated.
docs/workflow/online-ddl.mdx	Documents the “interrupted apply / added by hand” scenario and re-validation behavior.

Review details

Files not reviewed (1)

• ir/queries/queries.sql.go: Generated file

• Files reviewed: 13/14 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[greptile-apps]

Greptile Summary

This PR restores interrupted PostgreSQL 18 native NOT NULL migrations by exposing pending validation state through inspection, generating the missing VALIDATE CONSTRAINT, and isolating that scan in its own transaction group.

• Adds catalog inspection and IR support for unvalidated native NOT NULL constraints.
• Treats a pending validation as a column difference and emits identifier-safe validation SQL.
• Adds planner isolation, regression fixtures, unit coverage, and workflow documentation.
• Keeps PostgreSQL 14–17 behavior compatible because those versions have no matching contype = 'n' rows.

Confidence Score: 5/5

The PR appears safe to merge; no actionable correctness, security, compatibility, or repository-rule violations were identified.

Catalog query generation, inspector conversion, diff behavior, identifier quoting, and planner transaction isolation are aligned, and the regression coverage exercises both validation emission and execution grouping.

Important Files Changed

Filename	Overview
ir/queries/queries.sql	Extends both column queries to expose an unvalidated PostgreSQL 18 native NOT NULL constraint while remaining compatible with older versions.
ir/inspector.go	Transfers the nullable catalog result into the column IR.
ir/ir.go	Adds derived per-column state for a pending native NOT NULL validation.
internal/diff/column.go	Makes pending validation affect comparison and renders the corresponding quoted VALIDATE statement.
internal/plan/rewrite.go	Marks the pending validation step as requiring its own transaction group.
internal/plan/rewrite_test.go	Verifies that pending NOT NULL validation is isolated from surrounding DDL.
docs/workflow/online-ddl.mdx	Documents recovery when a native NOT NULL validation was interrupted or omitted.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    PG[(PostgreSQL catalogs)] --> Q[Inspect pg_constraint]
    Q --> IR[Column pending-validation state]
    IR --> D{Desired column is NOT NULL?}
    D -->|Yes, validation pending| SQL[Emit VALIDATE CONSTRAINT]
    SQL --> G[Isolated transaction group]
    G --> A[Apply validation scan]
    A --> C[Converged schema]

Loading

_{Reviews (1): Last reviewed commit: "fix: re-emit VALIDATE for an unvalidated..." | Re-trigger Greptile}