Skip to content

fix: support CHECK constraints without columns and NO INHERIT modifier (#386)#391

Merged
tianzhou merged 1 commit intomainfrom
fix/issue-386-check-no-inherit
Apr 7, 2026
Merged

fix: support CHECK constraints without columns and NO INHERIT modifier (#386)#391
tianzhou merged 1 commit intomainfrom
fix/issue-386-check-no-inherit

Conversation

@tianzhou
Copy link
Copy Markdown
Contributor

@tianzhou tianzhou commented Apr 7, 2026

Summary

  • Column-less CHECK constraints (e.g., CHECK (FALSE) NO INHERIT) were silently dropped during introspection because the LEFT JOIN on pg_attribute produced NULL for attname, triggering the columnName=="" guard
  • The NO INHERIT modifier was not tracked anywhere in the IR, queries, or DDL output
  • Added connoinherit to constraint queries, NoInherit field to IR, and NO INHERIT output in diff/plan/rewrite code paths

Fixes #386

Test plan

  • New test case: testdata/diff/online/issue_386_check_no_inherit/ with CHECK (false) NO INHERIT
  • All online diff tests pass (PGSCHEMA_TEST_FILTER="online/" go test -v ./internal/diff -run TestDiffFromFiles)
  • All integration tests pass (go test ./cmd -run TestPlanAndApply)
  • All dump tests pass (go test ./cmd/dump)

🤖 Generated with Claude Code

@tianzhou @claude
fix: support CHECK constraints without columns and NO INHERIT modifier (
bae3731 
#386)

Column-less CHECK constraints (e.g., CHECK (FALSE) NO INHERIT) were silently
dropped during introspection because the LEFT JOIN on pg_attribute produced
NULL for attname, triggering the columnName=="" guard. Additionally, the
NO INHERIT modifier was not tracked in the IR or output.

Fixes #386

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 7, 2026 10:58
Copilot AI reviewed Apr 7, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 schema introspection and DDL generation for PostgreSQL CHECK constraints that (1) reference no columns (e.g., CHECK (false)) and (2) use the NO INHERIT modifier, ensuring these constraints are represented in the IR and preserved through diff/plan/apply.

Changes:

  • Teach constraint introspection to retain column-less CHECK/EXCLUDE constraints and capture pg_constraint.connoinherit.
  • Extend IR + SQL generation paths to track and emit NO INHERIT for CHECK constraints.
  • Add an online diff fixture for Issue #386 and refresh plan fingerprints across existing fixtures.

Reviewed changes

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

Show a summary per file
File Description
ir/queries/queries.sql Adds connoinherit AS no_inherit to constraint queries.
ir/queries/queries.sql.go Regenerates query bindings to include NoInherit scanning/struct fields.
ir/ir.go Adds Constraint.NoInherit to IR.
ir/inspector.go Stops skipping column-less CHECK/EXCLUDE constraints; propagates NoInherit into IR.
ir/normalize.go Attempts to strip NO INHERIT/NOT VALID suffixes from pg_get_constraintdef output for normalization.
internal/diff/constraint.go Emits NO INHERIT for CHECK constraints and compares NoInherit in equality.
internal/diff/table.go Appends NO INHERIT when generating ALTER TABLE ADD CONSTRAINT for CHECK constraints.
internal/plan/rewrite.go Appends NO INHERIT in online rewrite SQL for CHECK constraints.
testdata/diff/online/issue_386_check_no_inherit/plan.txt New expected human plan output for Issue #386 fixture.
testdata/diff/online/issue_386_check_no_inherit/plan.sql New expected SQL plan output for Issue #386 fixture.
testdata/diff/online/issue_386_check_no_inherit/plan.json New expected JSON plan output for Issue #386 fixture.
testdata/diff/online/issue_386_check_no_inherit/old.sql New “old schema” input for Issue #386 fixture.
testdata/diff/online/issue_386_check_no_inherit/new.sql New “new schema” input for Issue #386 fixture.
testdata/diff/online/issue_386_check_no_inherit/diff.sql New expected diff output for Issue #386 fixture.
testdata/diff/privilege/revoke_table_privilege/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/privilege/revoke_grant_option/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/privilege/grant_with_grant_option/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/privilege/grant_table_select/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/privilege/grant_table_multiple/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/privilege/alter_privilege/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/online/issue_313_camelcase_column_not_null/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/online/issue_286_reserved_keyword_quoting/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/online/alter_fk/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/online/add_fk/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/migrate/v5/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/migrate/v4/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/migrate/v3/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/migrate/v2/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/dependency/table_to_view/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/dependency/table_to_materialized_view/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/dependency/issue_308_view_select_star_column_reorder/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/dependency/function_to_trigger/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/default_privilege/auto_grant_idempotent/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_view/issue_350_view_options/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_view/drop_view/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_view/alter_view/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_view/add_view/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_view/add_view_join/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_trigger/drop_trigger/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_trigger/alter_trigger/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_trigger/add_trigger/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_trigger/add_trigger_when_distinct/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_trigger/add_trigger_system_catalog/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_trigger/add_trigger_old_table/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_trigger/add_trigger_constraint/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_table/issue_384_rename_column_constraint/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_table/issue_382_drop_table_cascade_constraint/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_table/issue_283_function_default_schema_qualifier/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_table/issue_281_exclude_constraint/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_table/composite_fk_column_order/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_table/add_fk/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_table/add_default_not_null/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_table/add_column_cross_schema_custom_type/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_table/add_check/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_policy/remove_force_rls/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_policy/issue_377_nested_function_in_policy/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_policy/force_rls/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_policy/enable_rls/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_policy/drop_policy/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_policy/disable_rls/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_policy/alter_policy_using/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_policy/alter_policy_roles/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_policy/alter_policy_name/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_policy/alter_policy_command/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_policy/add_policy/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_materialized_view/drop_materialized_view/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_materialized_view/alter_materialized_view/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_materialized_view/add_materialized_view/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/create_function/issue_354_empty_search_path/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/comment/noop_column_comments/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/comment/mixed_comments/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/comment/drop_table_comment/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/comment/alter_table_comment/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/comment/add_table_comment/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/comment/add_index_comment/plan.json Updates fingerprint hash due to IR/plan serialization changes.
testdata/diff/comment/add_column_comments/plan.json Updates fingerprint hash due to IR/plan serialization changes.
Comments suppressed due to low confidence (1)

ir/normalize.go:1195

  • normalizeCheckClause strips the " NO INHERIT" suffix before stripping " NOT VALID". If pg_get_constraintdef returns the standard order "... NO INHERIT NOT VALID", the first HasSuffix(" NO INHERIT") check won’t match, and after trimming " NOT VALID" the remaining " NO INHERIT" is never removed. That can leave NO INHERIT inside CheckClause and then later DDL generation appends NO INHERIT again (and may even generate invalid CHECK syntax). Consider stripping suffixes in reverse order (NOT VALID first, then NO INHERIT) or looping until neither suffix is present so both orders are handled safely.
	// Strip " NOT VALID" and " NO INHERIT" suffixes if present
	// PostgreSQL's pg_get_constraintdef may include these at the end,
	// but we control their placement via the IsValid and NoInherit fields
	clause := strings.TrimSpace(checkClause)
	if strings.HasSuffix(clause, " NO INHERIT") {
		clause = strings.TrimSuffix(clause, " NO INHERIT")
		clause = strings.TrimSpace(clause)
	}
	if strings.HasSuffix(clause, " NOT VALID") {
		clause = strings.TrimSuffix(clause, " NOT VALID")
		clause = strings.TrimSpace(clause)
	}

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

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Apr 7, 2026

Greptile Summary

This PR fixes two related bugs in CHECK constraint handling: column-less CHECK (FALSE) constraints were silently dropped during introspection because the LEFT JOIN on pg_attribute produced NULL column names that triggered an unconditional continue; and the NO INHERIT modifier was never tracked in the IR, queries, or DDL output. The fix wires connoinherit end-to-end from the SQL query through the IR struct, normalization, and all three DDL emission paths (CREATE TABLE inline, ALTER TABLE ADD CONSTRAINT, and the online NOT VALID rewrite).

Confidence Score: 5/5

Safe to merge — all DDL paths emit NO INHERIT consistently and column-less constraints are no longer silently dropped.

The changes are internally consistent across all layers: connoinherit flows from both SQL queries through the generated row structs, IR struct, normalizer, diff generation, and plan rewriting. The constraintsEqual check includes NoInherit. The new test fixture covers the full online migration plan with NO INHERIT NOT VALID followed by VALIDATE CONSTRAINT. No P0 or P1 issues found.

No files require special attention.

Important Files Changed

Filename Overview
ir/ir.go Adds NoInherit bool field to Constraint struct with json:"no_inherit,omitempty" tag
ir/inspector.go Fixes silent drop of column-less CHECK constraints; wires NoInherit from connoinherit query field on constraint creation
ir/normalize.go Extends normalizeCheckClause to strip NO INHERIT suffix (in addition to NOT VALID) before removing CHECK prefix and outer parens
ir/queries/queries.sql Adds c.connoinherit AS no_inherit to both GetConstraints and GetConstraintsForSchema queries
ir/queries/queries.sql.go Adds NoInherit bool to generated row structs and scan calls for both constraint query functions
internal/diff/constraint.go Emits NO INHERIT after CHECK clause in generateConstraintSQL and adds NoInherit comparison to constraintsEqual
internal/diff/table.go Appends NO INHERIT suffix in ALTER TABLE ADD CONSTRAINT SQL for both online and non-online constraint-add paths
internal/plan/rewrite.go Appends NO INHERIT before NOT VALID in generateConstraintRewrite online migration SQL
testdata/diff/online/issue_386_check_no_inherit/new.sql New test fixture: desired state table with CHECK (false) NO INHERIT constraint
testdata/diff/online/issue_386_check_no_inherit/plan.json Expected plan: two-step online migration — ADD CONSTRAINT NOT VALID then VALIDATE CONSTRAINT

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[pg_constraint row
connoinherit=true, column=NULL] -->|columnName empty| B{constraintType?}
    B -->|CHECK or EXCLUDE| C[Build Constraint IR
NoInherit=connoinherit]
    B -->|PK / UNIQUE / FK| D[skip row]
    C --> E[normalizeCheckClause
strip NO INHERIT suffix
strip NOT VALID suffix]
    E --> F[CheckClause stores
clean expression only]
    C --> G[NoInherit bool
on IR Constraint]
    F & G --> H{DDL path}
    H -->|CREATE TABLE inline| I[generateConstraintSQL
CONSTRAINT x CHECK expr NO INHERIT]
    H -->|ALTER TABLE add| J[generateAlterTableStatements
ALTER TABLE t
ADD CONSTRAINT x CHECK expr NO INHERIT]
    H -->|Online migration| K[generateConstraintRewrite
ADD CONSTRAINT x CHECK expr NO INHERIT NOT VALID
VALIDATE CONSTRAINT x]
Loading

Reviews (1): Last reviewed commit: "fix: support CHECK constraints without c..." | Re-trigger Greptile

@tianzhou tianzhou merged commit c6ca756 into main Apr 7, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

CHECK (FALSE) NO INHERIT constraints silently dropped during plan/apply

2 participants

@tianzhou