fix: support NULLS NOT DISTINCT on unique indexes (#355)#356
Conversation
Add support for PostgreSQL 15+ NULLS NOT DISTINCT clause on unique indexes. Previously, this clause was silently dropped during schema inspection and plan generation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds support for PostgreSQL 15+ NULLS NOT DISTINCT on unique indexes by tracking the attribute in IR, detecting it during inspection, and emitting it in generated DDL so diffs/plans preserve the clause (fixes #355).
Changes:
- Added
NullsNotDistincttoir.Indexand included it in structural equality. - Detected
NULLS NOT DISTINCTfrompg_get_indexdef()output during inspection. - Emitted
NULLS NOT DISTINCTin both diff DDL generation and plan rewrite index SQL generation, and updated test fixtures accordingly.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| testdata/diff/create_index/add_index/new.sql | Adds a unique index fixture using NULLS NOT DISTINCT. |
| testdata/diff/create_index/add_index/diff.sql | Expected diff SQL now includes NULLS NOT DISTINCT. |
| testdata/diff/create_index/add_index/plan.sql | Expected plan SQL now includes NULLS NOT DISTINCT. |
| testdata/diff/create_index/add_index/plan.txt | Expected human-readable plan output includes the new index. |
| testdata/diff/create_index/add_index/plan.json | Expected JSON plan includes the new index creation SQL. |
| ir/ir.go | Extends Index IR with NullsNotDistinct. |
| ir/inspector.go | Detects NULLS NOT DISTINCT in inspected index definitions. |
| internal/diff/index.go | Emits NULLS NOT DISTINCT in CREATE INDEX DDL generation. |
| internal/diff/table.go | Treats NullsNotDistinct changes as requiring index recreation. |
| internal/plan/rewrite.go | Emits NULLS NOT DISTINCT in plan rewrite index SQL generation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| -- Test index name with dots (issue #196) | ||
| CREATE INDEX "public.idx_users" ON public.users (email, name); | ||
| -- Test NULLS NOT DISTINCT (issue #355) | ||
| CREATE UNIQUE INDEX idx_users_email_unique ON public.users (email) NULLS NOT DISTINCT; |
ir/inspector.go
Outdated
| // Check for NULLS NOT DISTINCT (PostgreSQL 15+) | ||
| if indexRow.Indexdef.Valid && strings.Contains(strings.ToUpper(indexRow.Indexdef.String), "NULLS NOT DISTINCT") { |
Greptile SummaryThis PR adds first-class support for PostgreSQL 15+ Key changes:
One style concern: the Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User SQL file with\nCREATE UNIQUE INDEX ... NULLS NOT DISTINCT] --> B[Embedded PostgreSQL\napplies SQL]
B --> C[Inspector: buildIndexes\npg_get_indexdef output]
C --> D{strings.Contains\nNULLS NOT DISTINCT?}
D -- Yes --> E[index.NullsNotDistinct = true]
D -- No --> F[index.NullsNotDistinct = false]
E --> G[IR: Index struct]
F --> G
G --> H{Diff path or\nPlan-rewrite path?}
H -- diff path --> I[indexesStructurallyEqual\nchecks NullsNotDistinct]
I -- changed --> J[DROP old index\nCREATE new index]
I -- unchanged --> K[no-op]
H -- generate DDL --> L[generateIndexSQLWithName\nor generateIndexSQL]
L --> M["...columns...\nNULLS NOT DISTINCT\nWHERE ..."]
M --> N[Final DDL output]
style E fill:#d4edda
style D fill:#fff3cd
style M fill:#d4edda
Last reviewed commit: d2ef09c |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // NULLS NOT DISTINCT for unique indexes (PostgreSQL 15+) | ||
| if index.NullsNotDistinct { | ||
| builder.WriteString(" NULLS NOT DISTINCT") | ||
| } |
There was a problem hiding this comment.
Missing uniqueness guard on NULLS NOT DISTINCT emission
The clause is emitted unconditionally for any index with NullsNotDistinct = true, but PostgreSQL only permits NULLS NOT DISTINCT on unique indexes. If an IR JSON file is loaded where a non-unique index has "nulls_not_distinct": true (e.g., a manually-edited dump), the generated DDL would be syntactically invalid and fail on apply.
Adding a uniqueness check makes the generation code self-defending:
| // NULLS NOT DISTINCT for unique indexes (PostgreSQL 15+) | |
| if index.NullsNotDistinct { | |
| builder.WriteString(" NULLS NOT DISTINCT") | |
| } | |
| // NULLS NOT DISTINCT for unique indexes (PostgreSQL 15+) | |
| if index.NullsNotDistinct && index.Type == ir.IndexTypeUnique { | |
| builder.WriteString(" NULLS NOT DISTINCT") | |
| } |
The same guard should be applied in internal/plan/rewrite.go line 414:
if index.NullsNotDistinct && index.Type == ir.IndexTypeUnique {
sql.WriteString(" NULLS NOT DISTINCT")
}Address review feedback: - Only emit NULLS NOT DISTINCT when index type is unique (defensive) - Skip create_index/add_index test on PG14 (NULLS NOT DISTINCT is PG15+) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds end-to-end support for PostgreSQL 15+ NULLS NOT DISTINCT on unique indexes so schema inspection, diffing, and plan/apply DDL generation preserve and recreate the clause when needed.
Changes:
- Extend
ir.Indexwith aNullsNotDistinctflag and populate it during inspection viapg_get_indexdef()output parsing. - Emit
NULLS NOT DISTINCTin both diff SQL generation and plan rewrite index SQL generation; include it in structural equality comparisons. - Update
create_index/add_indexfixtures to cover the new clause and skip that test on PostgreSQL 14.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| testutil/skip_list.go | Adds PG14-only skip list entry for the new PG15+ index syntax fixture. |
| testdata/diff/create_index/add_index/plan.txt | Updates expected plan output to include the new unique index. |
| testdata/diff/create_index/add_index/plan.sql | Updates expected plan SQL to include NULLS NOT DISTINCT. |
| testdata/diff/create_index/add_index/plan.json | Updates expected JSON plan steps to include the new index DDL. |
| testdata/diff/create_index/add_index/new.sql | Adds a NULLS NOT DISTINCT unique index to the input schema fixture. |
| testdata/diff/create_index/add_index/diff.sql | Updates expected diff SQL to include NULLS NOT DISTINCT. |
| ir/ir.go | Adds NullsNotDistinct to ir.Index for representing PG15+ behavior. |
| ir/inspector.go | Detects NULLS NOT DISTINCT from pg_get_indexdef() and sets the IR flag. |
| internal/plan/rewrite.go | Emits NULLS NOT DISTINCT when generating CREATE UNIQUE INDEX SQL in the rewrite path. |
| internal/diff/table.go | Includes NullsNotDistinct in index structural equality checks. |
| internal/diff/index.go | Emits NULLS NOT DISTINCT when generating CREATE UNIQUE INDEX SQL in the diff path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| // skipListPG14 defines test cases that should be skipped for PostgreSQL 14 only. | ||
| // | ||
| // Reason for skipping: | ||
| // These tests use features not available in PostgreSQL 14 (e.g., NULLS NOT DISTINCT is PG15+). | ||
| var skipListPG14 = []string{ | ||
| "create_index/add_index", | ||
| } | ||
|
|
||
| // skipListForVersion maps PostgreSQL major versions to their skip lists. | ||
| var skipListForVersion = map[int][]string{ | ||
| 14: skipListPG14_15, | ||
| 14: append(skipListPG14_15, skipListPG14...), | ||
| 15: skipListPG14_15, | ||
| } |
| // Check for NULLS NOT DISTINCT (PostgreSQL 15+) | ||
| if indexRow.Indexdef.Valid && strings.Contains(strings.ToUpper(indexRow.Indexdef.String), "NULLS NOT DISTINCT") { | ||
| index.NullsNotDistinct = true |
- pg_get_indexdef() always returns uppercase, no need for ToUpper - Use generic skip message for version-based test skips Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds end-to-end support for PostgreSQL 15+ NULLS NOT DISTINCT on unique indexes by tracking the property in IR, detecting it during inspection, and emitting it in both diff and plan-rewrite SQL generation so it’s not silently dropped.
Changes:
- Extend
ir.Indexwith aNullsNotDistinctflag and detect it frompg_get_indexdef()during inspection. - Emit
NULLS NOT DISTINCTwhen generatingCREATE UNIQUE INDEXSQL in both diff and plan rewrite paths; include it in structural equality to force index recreation on changes. - Update diff test fixtures to include a
NULLS NOT DISTINCTunique index and skip the affected case on PG14.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
ir/ir.go |
Adds NullsNotDistinct to the index IR model. |
ir/inspector.go |
Sets NullsNotDistinct based on pg_get_indexdef() output. |
internal/diff/index.go |
Renders NULLS NOT DISTINCT for unique index DDL in diff generation. |
internal/diff/table.go |
Includes NullsNotDistinct in structural equality checks to trigger recreation. |
internal/plan/rewrite.go |
Renders NULLS NOT DISTINCT for unique index DDL in rewrite generation. |
testdata/diff/create_index/add_index/* |
Updates expected plans/diffs and adds a fixture index using NULLS NOT DISTINCT. |
testutil/skip_list.go |
Adds a PG14-only skip list entry for the new fixture (feature unavailable on PG14). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Copy skipListPG14_15 before appending PG14-only entries to prevent cross-version bleed-through if the backing array has spare capacity. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds end-to-end support for PostgreSQL 15+ NULLS NOT DISTINCT on unique indexes by persisting the attribute in IR, detecting it during inspection, and emitting it consistently in diff/plan SQL output so schema changes recreate indexes when needed.
Changes:
- Extend
ir.IndexwithNullsNotDistinctand set it during inspection by scanningpg_get_indexdef()output. - Emit
NULLS NOT DISTINCTin both diff DDL generation and plan rewrite index SQL generation paths. - Update index structural equality to include
NullsNotDistinct, and update test fixtures + PG14 skipping for the new PG15-only feature.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
testutil/skip_list.go |
Skips the updated create_index/add_index fixture on PG14 since the fixture now contains a PG15+ index feature. |
testdata/diff/create_index/add_index/plan.txt |
Fixture output updated to include the new unique index. |
testdata/diff/create_index/add_index/plan.sql |
Fixture output updated to include NULLS NOT DISTINCT in generated SQL. |
testdata/diff/create_index/add_index/plan.json |
Fixture output updated to include the new index SQL in JSON plan. |
testdata/diff/create_index/add_index/new.sql |
Adds a NULLS NOT DISTINCT unique index to the test schema. |
testdata/diff/create_index/add_index/diff.sql |
Fixture output updated to include NULLS NOT DISTINCT. |
ir/ir.go |
Adds the NullsNotDistinct field to the Index IR model. |
ir/inspector.go |
Detects NULLS NOT DISTINCT via pg_get_indexdef() output during index inspection. |
internal/plan/rewrite.go |
Emits NULLS NOT DISTINCT when generating index SQL in the rewrite path. |
internal/diff/table.go |
Includes NullsNotDistinct in structural index equality to force recreation on changes. |
internal/diff/index.go |
Emits NULLS NOT DISTINCT in diff-generated CREATE UNIQUE INDEX statements. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| IsPartial bool `json:"is_partial"` // has a WHERE clause | ||
| IsExpression bool `json:"is_expression"` // functional/expression index | ||
| Where string `json:"where,omitempty"` // partial index condition | ||
| NullsNotDistinct bool `json:"nulls_not_distinct,omitempty"` // NULLS NOT DISTINCT (PG15+) | ||
| Comment string `json:"comment,omitempty"` |
There was a problem hiding this comment.
Pull request overview
Adds end-to-end support for PostgreSQL 15+ NULLS NOT DISTINCT on unique indexes so the inspector captures it, diffs treat it as a structural change, and both diff and plan DDL generators emit the clause.
Changes:
- Extend
ir.IndexwithNullsNotDistinctand populate it during inspection usingpg_get_indexdef(). - Emit
NULLS NOT DISTINCTin both diff SQL generation and plan rewrite SQL generation (unique indexes only). - Update the
create_index/add_indexdiff fixtures and skip that test on PG14 (feature not supported).
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
ir/ir.go |
Adds NullsNotDistinct to the index IR model. |
ir/inspector.go |
Detects NULLS NOT DISTINCT from pg_get_indexdef() output and stores it in IR. |
internal/diff/table.go |
Includes NullsNotDistinct in structural equality so diffs trigger index recreation when it changes. |
internal/diff/index.go |
Emits NULLS NOT DISTINCT for unique indexes in diff-generated CREATE UNIQUE INDEX. |
internal/plan/rewrite.go |
Emits NULLS NOT DISTINCT for unique indexes in plan rewrite CREATE UNIQUE INDEX. |
testdata/diff/create_index/add_index/* |
Updates golden fixtures to include the new unique index with NULLS NOT DISTINCT. |
testutil/skip_list.go |
Skips create_index/add_index on PG14 to avoid running PG15+ syntax in PG14 test runs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Summary
NullsNotDistinctfield toir.Indexstruct to track PostgreSQL 15+ NULLS NOT DISTINCT indexespg_get_indexdef()output during inspection (no version-conditional SQL needed)NULLS NOT DISTINCTin both diff and plan rewrite DDL generation pathsFixes #355
Test plan
create_index/add_indextest casePGSCHEMA_TEST_FILTER="create_index/" go test -v ./internal/diff -run TestDiffFromFilespassesPGSCHEMA_TEST_FILTER="create_index/" go test -v ./cmd -run TestPlanAndApplypassesgo vet ./...clean🤖 Generated with Claude Code