Skip to content

fix: run VALIDATE CONSTRAINT in its own transaction for online rewrites - #565

Merged
tianzhou merged 1 commit into
mainfrom
fix-online-rewrite-transaction-isolation
Aug 30, 2026
Merged

fix: run VALIDATE CONSTRAINT in its own transaction for online rewrites#565
tianzhou merged 1 commit into
mainfrom
fix-online-rewrite-transaction-isolation

Conversation

@tianzhou

Copy link
Copy Markdown
Contributor

Problem

The online rewrites for CHECK constraints, foreign keys, and SET NOT NULL emit an ADD ... NOT VALID / VALIDATE CONSTRAINT pair, but every step was marked transactional, so apply concatenated them into one implicit transaction. The ACCESS EXCLUSIVE (or SHARE ROW EXCLUSIVE for FKs) lock taken by ADD CONSTRAINT was held through the VALIDATE full-table scan, blocking reads and writes for the entire scan — the rewrite provided no online benefit over the plain DDL it replaces. The NOT VALID/VALIDATE pattern only works when the two statements commit in separate transactions.

Fix

  • Add RequiresIsolation to RewriteStep and set it on the VALIDATE CONSTRAINT step in all three rewrites (generateConstraintRewrite, generateForeignKeyRewrite, generateColumnNotNullRewrite).
  • Plan grouping (groupDiffs) now places such steps in their own execution group, i.e. their own implicit transaction.

Resulting execution shape:

  1. ADD ... NOT VALID commits with the preceding quick steps (for constraint replacements, DROP + ADD ... NOT VALID stay atomic in one group)
  2. VALIDATE CONSTRAINT runs alone, scanning under SHARE UPDATE EXCLUSIVE without blocking traffic
  3. Follow-up steps (SET NOT NULL, DROP CONSTRAINT) batch with subsequent transactional steps

plan.txt renders the new boundaries as -- Transaction Group #N, so the split is visible at plan review time.

Trade-off

Previously a plan containing these rewrites was all-or-nothing. Now, if VALIDATE fails (e.g. existing rows violate the constraint), the already-committed NOT VALID constraint remains in place. This is inherent to online DDL, matches how CREATE INDEX CONCURRENTLY already behaves (partial state on failure), and the fingerprint-based re-plan flow recovers from it.

Docs

  • docs/workflow/online-ddl.mdx: documents the separate-transaction guarantee
  • docs/cli/apply.mdx: corrects the claim that all changes run in a single transaction

Testing

  • Regenerated the 14 affected fixture sets (online/, migrate/, dependency/issue_439, five create_table/ cases)
  • go test ./internal/plan ./internal/diff — pass
  • TestPlanAndApply integration runs against embedded PostgreSQL for every affected category — pass

🤖 Generated with Claude Code

The online rewrites for CHECK constraints, foreign keys, and SET NOT
NULL emit an ADD ... NOT VALID / VALIDATE CONSTRAINT pair, but all
steps were marked transactional, so apply batched them into a single
implicit transaction. The strong lock taken by ADD was then held
through the VALIDATE full-table scan, blocking reads and writes for
the entire scan - no better than the plain DDL the rewrite replaces.

Add RequiresIsolation to RewriteStep and set it on VALIDATE steps so
plan grouping puts them in their own execution group (their own
implicit transaction). ADD ... NOT VALID now commits first (for
constraint replacements, DROP + ADD stay atomic in one group), then
VALIDATE scans under its weaker SHARE UPDATE EXCLUSIVE lock without
blocking traffic.

plan.txt now shows the boundaries as "-- Transaction Group #N", and
the docs note the separate-transaction guarantee. Trade-off: if
VALIDATE fails (existing rows violate the constraint), the committed
NOT VALID constraint remains; the fingerprint-based re-plan flow
recovers from that, matching how CONCURRENTLY index builds already
behave.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 30, 2026 13:22
@greptile-apps

greptile-apps Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR separates online constraint validation from the preceding ADD ... NOT VALID, allowing PostgreSQL to release the stronger initial DDL lock before scanning existing rows.

  • Adds an internal isolation marker to CHECK, foreign-key, and NOT NULL rewrite validation steps.
  • Updates execution grouping so isolated validation steps run in independent implicit transactions.
  • Regenerates affected plan fixtures to expose the new transaction boundaries.
  • Updates apply and online-DDL documentation to describe grouped rollback and partial-state behavior.

Confidence Score: 5/5

The PR appears safe to merge; the new execution groups produce the intended independent commits and the documented recovery path handles validation failures.

Apply executes each serialized directive-free group as a separate autocommit call, while fresh planning recognizes and replaces any partially applied NOT VALID constraint without duplicate-name or omitted-validation failures.

Important Files Changed

Filename Overview
internal/plan/plan.go Extends execution grouping to flush pending work and place isolation-marked rewrite steps in singleton groups.
internal/plan/rewrite.go Marks CHECK, foreign-key, and NOT NULL validation steps for isolated transactional execution.
docs/cli/apply.mdx Correctly documents group-level rollback instead of claiming that every migration is globally atomic.
docs/workflow/online-ddl.mdx Documents why staged constraint addition and validation require separate transactions.
testdata/diff/online/add_constraint/plan.json Captures the serialized transaction boundary between adding and validating an online CHECK constraint.
testdata/diff/online/add_fk/plan.json Captures independent transaction groups for online foreign-key validation.
testdata/diff/online/add_not_null/plan.json Captures separate add, validation, and follow-up groups for the NOT NULL rewrite.

Sequence Diagram

sequenceDiagram
  participant P as Planner
  participant A as Apply
  participant DB as PostgreSQL
  P->>A: Group 1: ADD CONSTRAINT ... NOT VALID
  A->>DB: Execute group in implicit transaction
  DB-->>A: Commit and release strong DDL lock
  P->>A: Group 2: VALIDATE CONSTRAINT
  A->>DB: Execute validation in separate transaction
  DB-->>A: Commit validated constraint
  P->>A: Group 3: Follow-up transactional steps
  A->>DB: SET NOT NULL / DROP temporary constraint
Loading

Reviews (1): Last reviewed commit: "fix: run VALIDATE CONSTRAINT in its own ..." | Re-trigger Greptile

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

This PR fixes an online-DDL correctness issue where ADD ... NOT VALID and VALIDATE CONSTRAINT were being executed within the same implicit transaction group, causing stronger locks to be held through the validation scan and negating the “online” benefit.

Changes:

  • Added a RequiresIsolation flag to RewriteStep and set it for VALIDATE CONSTRAINT steps in CHECK, FK, and SET NOT NULL online rewrites.
  • Updated plan grouping logic to force isolation-required steps into their own execution group (separate implicit transaction).
  • Updated docs and regenerated affected plan fixtures to reflect the new transaction boundaries.

Reviewed changes

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

Show a summary per file
File Description
internal/plan/rewrite.go Adds RequiresIsolation and marks VALIDATE CONSTRAINT steps as isolated in constraint/foreign key/NOT NULL rewrites.
internal/plan/plan.go Extends grouping logic to split isolated rewrite steps into their own execution group/transaction boundary.
docs/workflow/online-ddl.mdx Documents that ADD ... NOT VALID and VALIDATE CONSTRAINT run in separate transactions for online constraint operations.
docs/cli/apply.mdx Updates transactional-mode description to reflect transaction-group execution and isolated-step behavior.
testdata/diff/online/issue_386_check_no_inherit/plan.txt Fixture update: shows transaction group split between ADD NOT VALID and VALIDATE.
testdata/diff/online/issue_386_check_no_inherit/plan.json Fixture update: isolates VALIDATE into its own execution group.
testdata/diff/online/issue_313_camelcase_column_not_null/plan.txt Fixture update: splits NOT NULL rewrite into 3 groups (ADD, VALIDATE, SET NOT NULL + cleanup).
testdata/diff/online/issue_313_camelcase_column_not_null/plan.json Fixture update: isolates VALIDATE into its own execution group.
testdata/diff/online/alter_fk/plan.txt Fixture update: splits FK replacement so VALIDATE runs in its own group.
testdata/diff/online/alter_fk/plan.json Fixture update: isolates VALIDATE into its own execution group.
testdata/diff/online/alter_constraint/plan.txt Fixture update: splits CHECK replacement so VALIDATE runs in its own group.
testdata/diff/online/alter_constraint/plan.json Fixture update: isolates VALIDATE into its own execution group.
testdata/diff/online/add_not_null/plan.txt Fixture update: splits NOT NULL rewrite steps into separate transaction groups.
testdata/diff/online/add_not_null/plan.json Fixture update: isolates VALIDATE into its own execution group.
testdata/diff/online/add_fk/plan.txt Fixture update: ensures each FK VALIDATE runs in its own group.
testdata/diff/online/add_fk/plan.json Fixture update: isolates VALIDATE steps into their own execution groups.
testdata/diff/online/add_constraint/plan.txt Fixture update: ensures each CHECK VALIDATE runs in its own group.
testdata/diff/online/add_constraint/plan.json Fixture update: isolates VALIDATE steps into their own execution groups.
testdata/diff/online/add_composite_index/plan.txt Fixture update (line-number normalization) as part of regenerated outputs.
testdata/diff/migrate/v2/plan.txt Fixture update: transaction group numbering/boundaries updated due to isolated VALIDATE steps.
testdata/diff/migrate/v2/plan.json Fixture update: isolates VALIDATE steps into their own execution groups.
testdata/diff/dependency/issue_439_unique_to_pk_fk_dependent/plan.txt Fixture update: isolates FK VALIDATE steps into separate groups.
testdata/diff/dependency/issue_439_unique_to_pk_fk_dependent/plan.json Fixture update: isolates VALIDATE steps into their own execution groups.
testdata/diff/create_table/issue_384_rename_column_constraint/plan.txt Fixture update: isolates FK VALIDATE after ADD NOT VALID into its own group.
testdata/diff/create_table/issue_384_rename_column_constraint/plan.json Fixture update: isolates VALIDATE into its own execution group.
testdata/diff/create_table/composite_fk_column_order/plan.txt Fixture update: isolates FK VALIDATE into its own group.
testdata/diff/create_table/composite_fk_column_order/plan.json Fixture update: isolates VALIDATE into its own execution group.
testdata/diff/create_table/add_fk/plan.txt Fixture update: isolates each FK VALIDATE into its own transaction group.
testdata/diff/create_table/add_fk/plan.json Fixture update: isolates VALIDATE steps into their own execution groups.
testdata/diff/create_table/add_default_not_null/plan.txt Fixture update: splits NOT NULL rewrite into separate groups including isolated VALIDATE.
testdata/diff/create_table/add_default_not_null/plan.json Fixture update: isolates VALIDATE into its own execution group.
testdata/diff/create_table/add_check/plan.txt Fixture update: isolates CHECK VALIDATE into its own group.
testdata/diff/create_table/add_check/plan.json Fixture update: isolates VALIDATE into its own execution group.

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

@tianzhou
tianzhou merged commit 68db06b into main Aug 30, 2026
3 checks passed
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.

2 participants