Skip to content

Commit 194f72e

Browse files
tianzhouclaude
andauthored
fix: table-level CHECK constraints with IS NOT NULL silently omitted from dump (#396) (#398)
The inspector was using strings.Contains(checkClause, "IS NOT NULL") to skip system-generated NOT NULL constraints, but this incorrectly filtered out any CHECK constraint whose expression contained IS NOT NULL — including complex user-defined constraints like CHECK (status = 'active' OR reason IS NOT NULL). Narrowed the filter to only skip simple single-identifier NOT NULL checks (e.g., CHECK ((value IS NOT NULL))) by stripping the CHECK wrapper and parentheses and verifying the prefix is a single word with no spaces. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 8b9d15d commit 194f72e

6 files changed

Lines changed: 108 additions & 3 deletions

File tree

cmd/dump/dump_integration_test.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,13 @@ func TestDumpCommand_Issue345ArrayCast(t *testing.T) {
130130
runExactMatchTest(t, "issue_345_array_cast")
131131
}
132132

133+
func TestDumpCommand_Issue396CheckConstraintIsNotNull(t *testing.T) {
134+
if testing.Short() {
135+
t.Skip("Skipping integration test in short mode")
136+
}
137+
runExactMatchTest(t, "issue_396_check_constraint_is_not_null")
138+
}
139+
133140
func TestDumpCommand_Issue191FunctionProcedureOverload(t *testing.T) {
134141
if testing.Short() {
135142
t.Skip("Skipping integration test in short mode")

ir/inspector.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -517,9 +517,18 @@ func (i *Inspector) buildConstraints(ctx context.Context, schema *IR, targetSche
517517
// Handle check constraints
518518
if cType == ConstraintTypeCheck {
519519
if checkClause := i.safeInterfaceToString(constraint.CheckClause); checkClause != "" && checkClause != "<nil>" {
520-
// Skip system-generated NOT NULL constraints as they're redundant with column definitions
521-
if strings.Contains(checkClause, "IS NOT NULL") {
522-
continue
520+
// Skip simple NOT NULL check constraints (e.g., "CHECK ((value IS NOT NULL))")
521+
// as they're redundant with column definitions. Only skip if the entire
522+
// expression is just "identifier IS NOT NULL".
523+
inner := strings.TrimPrefix(strings.TrimSpace(checkClause), "CHECK ")
524+
inner = strings.TrimSpace(inner)
525+
for len(inner) > 2 && inner[0] == '(' && inner[len(inner)-1] == ')' && isBalancedParentheses(inner[1:len(inner)-1]) {
526+
inner = strings.TrimSpace(inner[1 : len(inner)-1])
527+
}
528+
if prefix, ok := strings.CutSuffix(inner, " IS NOT NULL"); ok {
529+
if !strings.Contains(strings.TrimSpace(prefix), " ") {
530+
continue
531+
}
523532
}
524533

525534
// Use CheckClause as-is from PostgreSQL's pg_get_constraintdef(c.oid, true)
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
"name": "issue_396_check_constraint_is_not_null",
3+
"description": "Table-level CHECK constraints containing IS NOT NULL in complex expressions are silently omitted from schema dump",
4+
"source": "https://github.com/pgplex/pgschema/issues/396",
5+
"notes": [
6+
"The inspector incorrectly skips CHECK constraints that contain 'IS NOT NULL' anywhere in the expression",
7+
"Only simple NOT NULL check constraints should be skipped, not complex expressions using IS NOT NULL"
8+
]
9+
}
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
--
2+
-- PostgreSQL database dump
3+
--
4+
5+
SET statement_timeout = 0;
6+
SET lock_timeout = 0;
7+
SET client_encoding = 'UTF8';
8+
SET standard_conforming_strings = on;
9+
SET check_function_bodies = false;
10+
SET client_min_messages = warning;
11+
SET row_security = off;
12+
13+
--
14+
-- Name: test_table; Type: TABLE; Schema: public; Owner: -
15+
--
16+
17+
CREATE TABLE public.test_table (
18+
id integer NOT NULL,
19+
status text NOT NULL,
20+
reason text,
21+
actor_id uuid
22+
);
23+
24+
--
25+
-- Name: test_table test_table_pkey; Type: CONSTRAINT; Schema: public; Owner: -
26+
--
27+
28+
ALTER TABLE ONLY public.test_table
29+
ADD CONSTRAINT test_table_pkey PRIMARY KEY (id);
30+
31+
--
32+
-- Name: test_table test_table_status_check; Type: CHECK CONSTRAINT; Schema: public; Owner: -
33+
--
34+
35+
ALTER TABLE public.test_table
36+
ADD CONSTRAINT test_table_status_check CHECK (((status = 'active'::text) OR ((status = 'cancelled'::text) AND (reason IS NOT NULL)) OR ((status = 'revoked'::text) AND (actor_id IS NOT NULL))));
37+
38+
--
39+
-- PostgreSQL database dump complete
40+
--
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
--
2+
-- pgschema database dump
3+
--
4+
5+
-- Dumped from database version PostgreSQL 18.0
6+
-- Dumped by pgschema version 1.9.0
7+
8+
9+
--
10+
-- Name: test_table; Type: TABLE; Schema: -; Owner: -
11+
--
12+
13+
CREATE TABLE IF NOT EXISTS test_table (
14+
id integer,
15+
status text NOT NULL,
16+
reason text,
17+
actor_id uuid,
18+
CONSTRAINT test_table_pkey PRIMARY KEY (id),
19+
CONSTRAINT test_table_status_check CHECK (status = 'active'::text OR status = 'cancelled'::text AND reason IS NOT NULL OR status = 'revoked'::text AND actor_id IS NOT NULL)
20+
);
21+
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
--
2+
-- Test case for GitHub issue #396: Table-level CHECK constraints omitted from schema dump
3+
--
4+
-- CHECK constraints containing IS NOT NULL in complex expressions
5+
-- are silently dropped because the inspector filters out any constraint
6+
-- with "IS NOT NULL" in its expression, not just simple NOT NULL constraints.
7+
--
8+
9+
CREATE TABLE test_table (
10+
id int PRIMARY KEY,
11+
status text NOT NULL,
12+
reason text,
13+
actor_id uuid,
14+
CONSTRAINT test_table_status_check CHECK (
15+
(status = 'active')
16+
OR (status = 'cancelled' AND reason IS NOT NULL)
17+
OR (status = 'revoked' AND actor_id IS NOT NULL)
18+
)
19+
);

0 commit comments

Comments
 (0)