feat: use native NOT NULL NOT VALID constraints on PostgreSQL 18+

[tianzhou]

Related to #564.

What

PostgreSQL 18 supports invalid NOT NULL constraints natively. On PG18+ targets, the online SET NOT NULL rewrite now emits:

ALTER TABLE users ADD CONSTRAINT users_email_not_null NOT NULL email NOT VALID;
ALTER TABLE users VALIDATE CONSTRAINT users_email_not_null;

instead of the four-step CHECK constraint pattern (add check NOT VALID → validate → SET NOT NULL → drop check). That's 2 statements and 1 ACCESS EXCLUSIVE acquisition instead of 4 and 3 — each avoided lock acquisition is one less lock-queue pileup hazard on a busy table. The VALIDATE step remains isolated in its own transaction (per #565), so the existing-row scan runs under SHARE UPDATE EXCLUSIVE without blocking traffic. New writes are enforced from the moment the NOT VALID constraint lands.

PostgreSQL 14–17 (and unknown versions) keep the portable CHECK pattern unchanged.

How

• The target's major version is parsed from the current-state IR's DatabaseVersion and threaded through NewPlan → groupDiffs → generateRewrite.
• The constraint name (<table>_<col>_not_null) matches what PostgreSQL auto-generates for a NOT NULL column in CREATE TABLE, so a migrated table converges with a freshly created one.
• No dump/diff drift either way: the inspector tracks nullability via attnotnull and skips contype='n' constraint rows entirely.

Verified against live PostgreSQL 18.1

• Syntax accepted; convalidated is f after ADD, t after VALIDATE
• New rows are rejected while the constraint is still NOT VALID
• attnotnull is set immediately on ADD
• CREATE TABLE (... col text NOT NULL) auto-names the constraint <table>_<col>_not_null, matching our chosen name

Behavior note

If apply is interrupted after the ADD but before VALIDATE, the column already reads as NOT NULL in the catalog (attnotnull = true) while old rows are unvalidated — a re-plan sees no drift and won't re-emit the VALIDATE. This edge is inherent to how PG18 models invalid NOT NULL constraints; new writes are enforced throughout, so the exposure is limited to pre-existing rows.

Testing

• New internal/plan/rewrite_test.go unit tests cover the PG18 native path, the PG17 fallback, the unknown-version fallback, and identifier quoting — important because integration golden files only run against the latest PostgreSQL version, so the pre-18 path has no golden coverage.
• Regenerated the three affected fixtures (online/add_not_null, online/issue_313_camelcase_column_not_null, create_table/add_default_not_null); regeneration applies the plans against embedded PG18, verifying the native path end-to-end. All suites re-run clean without --generate.
• Docs: online-ddl.mdx now documents both version paths (and corrects the old three-step description, which omitted the final DROP CONSTRAINT).

🤖 Generated with Claude Code

[greptile-apps]

Greptile Summary

The PR gates SET NOT NULL planning on the target PostgreSQL major version and introduces PostgreSQL 18's native NOT NULL NOT VALID sequence while retaining the four-step CHECK fallback for older or unknown versions.

• Threads target-version metadata through plan construction and rewrite selection.
• Emits isolated ADD and VALIDATE execution groups for PostgreSQL 18.
• Adds version-path and identifier-quoting tests, regenerates PostgreSQL 18 fixtures, and updates online-DDL documentation.

Confidence Score: 4/5

The PR should not merge until the PostgreSQL 18 rewrite handles existing constraints that already occupy its generated name.

A valid nullable-to-NOT-NULL migration can emit an ADD CONSTRAINT name already retained on the table, causing PostgreSQL to reject the plan before validation.

Files Needing Attention: internal/plan/rewrite.go

Important Files Changed

Filename	Overview
cmd/plan/plan.go	Parses the inspected target version and passes its major version into plan construction, with a safe portable fallback when unknown.
internal/plan/plan.go	Threads the target major version through grouping while preserving isolation boundaries for validation steps.
internal/plan/rewrite.go	Adds the PostgreSQL 18 native rewrite, but its fixed constraint name can collide with a retained existing constraint and abort apply.
internal/plan/rewrite_test.go	Covers PostgreSQL 18, older, unknown-version, and quoting behavior but does not cover occupied generated constraint names.
docs/workflow/online-ddl.mdx	Documents both version-dependent NOT NULL rewrite sequences and their transaction behavior.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Inspect target PostgreSQL version] --> B{Major version at least 18?}
    B -->|Yes| C[ADD native NOT NULL constraint NOT VALID]
    C --> D[Commit ADD group]
    D --> E[VALIDATE CONSTRAINT in isolated group]
    B -->|No or unknown| F[ADD temporary CHECK NOT VALID]
    F --> G[VALIDATE CHECK in isolated group]
    G --> H[SET NOT NULL]
    H --> I[DROP temporary CHECK]

Loading

_{Reviews (1): Last reviewed commit: "feat: use native NOT NULL NOT VALID cons..." | Re-trigger Greptile}

[Copilot]

Pull request overview

This PR updates pgschema’s online SET NOT NULL rewrite to take advantage of PostgreSQL 18’s native NOT NULL ... NOT VALID constraints, reducing both statement count and heavyweight lock acquisitions while preserving the existing portable behavior on PostgreSQL 14–17 and on unknown versions.

Changes:

• Thread the target PostgreSQL major version into plan generation (cmd/plan → internal/plan) to gate version-specific rewrites.
• For PG18+, rewrite SET NOT NULL to ADD CONSTRAINT ... NOT NULL ... NOT VALID + isolated VALIDATE CONSTRAINT (instead of the CHECK-constraint 4-step pattern).
• Update fixtures, add focused unit tests for both version paths, and document both behaviors.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
cmd/plan/plan.go	Parses target major version from current-state IR metadata and passes it into plan construction to gate rewrites.
internal/plan/plan.go	Threads targetMajorVersion through plan grouping and rewrite generation.
internal/plan/rewrite.go	Implements PG18+ native NOT NULL constraint rewrite and preserves pre-18 CHECK-based fallback.
internal/plan/rewrite_test.go	Adds unit coverage for PG18/native, PG17 fallback, unknown-version fallback, and quoting.
internal/plan/plan_test.go	Updates tests to the new NewPlan signature.
docs/workflow/online-ddl.mdx	Documents both PG18+ and PG14–17 NOT NULL strategies and corrects the earlier missing drop step.
testdata/diff/online/add_not_null/plan.{txt,sql,json}	Regenerated expected plans for the PG18+ native rewrite path.
testdata/diff/online/issue_313_camelcase_column_not_null/plan.{txt,sql,json}	Regenerated expected plans for quoted identifiers under the PG18+ native rewrite path.
testdata/diff/create_table/add_default_not_null/plan.{txt,sql,json}	Regenerated expected plans where NOT NULL rewrite interacts with other column changes.

Suppressed comments (1)

docs/workflow/online-ddl.mdx:122

• In the PG14–17 example, the comment “Add check constraint (non-blocking)” is misleading: ALTER TABLE ... ADD CONSTRAINT ... NOT VALID still needs to acquire an ACCESS EXCLUSIVE lock (typically briefly). Consider rewording to reflect that it’s fast but may momentarily block while the lock is taken.

On PostgreSQL 14-17, adding `NOT NULL` uses a check constraint based process:

```sql
-- 1. Add check constraint (non-blocking)
ALTER TABLE users ADD CONSTRAINT email_not_null CHECK (email IS NOT NULL) NOT VALID;

---

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

[jozef-slezak]

Thank you for a very quick reaction. Maybe one hing:

a re-plan sees no drift and won't re-emit the VALIDATE
Whan about selecting pg_constraint not null?

SELECT
    conname,
    conrelid::regclass AS table_name,
    convalidated,
    connoinherit
FROM pg_constraint
WHERE contype = 'n';

[tianzhou]

The greptile finding is confirmed and fixed in afae6f9 — reproduced against live PostgreSQL 18.1 before fixing (apply aborted with SQLSTATE 42710).

Two things the review surface-level framing missed, for the record:

1. The collision was guaranteed, not incidental. The realistic occupant of the name is a leftover CHECK (col IS NOT NULL) from a manual online migration. The inspector deliberately hides such checks from the IR (ir/inspector.go, "redundant with column definitions"), so the diff never emits a DROP CONSTRAINT to free the name.
2. The pre-18 path had the same latent bug — the four-step dance hardcoded <col>_not_null with no collision handling since it shipped.

The fix addresses both paths: the inspector now records every constraint name present on each table (including rows it otherwise skips — PG18 contype='n' rows, redundant IS NOT NULL checks, ignored constraints) in a non-serialized Table.AllConstraintNames set, and the rewrites pick the first free name with an integer suffix (users_email_not_null1, ...) the way PostgreSQL's own ChooseConstraintName does. Fingerprints and plan JSON are unaffected (json:"-", verified via the fingerprint suite).

Covered by a new regression fixture (online/not_null_name_collision) that plans and applies end-to-end against embedded PG18, plus unit tests for the PG18 path, the pre-18 path, and suffix exhaustion.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

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

[tianzhou]

@jozef-slezak Exactly right — that's the correct catalog signal. It matches what we verified on live 18.1 while building this: after ADD ... NOT NULL ... NOT VALID, the constraint row has contype = 'n', convalidated = f, while pg_attribute.attnotnull is already t — and attnotnull is what the inspector currently derives nullability from, which is why a re-plan sees no drift.

Making the plan self-healing the way you describe (inspector reads convalidated on contype='n' rows, IR represents "NOT NULL but unvalidated", diff emits just the missing VALIDATE CONSTRAINT) is a well-scoped enhancement, but it touches IR shape and diff logic, so we'll take it as a follow-up rather than growing this PR. Worth noting the exposure window is narrow: it only arises if apply dies between the two transaction groups, and new writes are enforced throughout — only pre-existing rows remain unvalidated until the VALIDATE runs.

🤖 Generated with Claude Code