diff --git a/cmd/dump/dump_integration_test.go b/cmd/dump/dump_integration_test.go index f1fb8209..7a79041b 100644 --- a/cmd/dump/dump_integration_test.go +++ b/cmd/dump/dump_integration_test.go @@ -137,6 +137,13 @@ func TestDumpCommand_Issue396CheckConstraintIsNotNull(t *testing.T) { runExactMatchTest(t, "issue_396_check_constraint_is_not_null") } +func TestDumpCommand_Issue412UniqueNullsNotDistinct(t *testing.T) { + if testing.Short() { + t.Skip("Skipping integration test in short mode") + } + runExactMatchTest(t, "issue_412_unique_nulls_not_distinct") +} + func TestDumpCommand_Issue191FunctionProcedureOverload(t *testing.T) { if testing.Short() { t.Skip("Skipping integration test in short mode") diff --git a/internal/diff/constraint.go b/internal/diff/constraint.go index 0c552b49..4846d624 100644 --- a/internal/diff/constraint.go +++ b/internal/diff/constraint.go @@ -32,7 +32,11 @@ func generateConstraintSQL(constraint *ir.Constraint, targetSchema string) strin if constraint.IsTemporal && len(cols) > 0 { cols[len(cols)-1] = cols[len(cols)-1] + " WITHOUT OVERLAPS" } - return fmt.Sprintf("CONSTRAINT %s UNIQUE (%s)", ir.QuoteIdentifier(constraint.Name), strings.Join(cols, ", ")) + modifier := "" + if constraint.NullsNotDistinct { + modifier = " NULLS NOT DISTINCT" + } + return fmt.Sprintf("CONSTRAINT %s UNIQUE%s (%s)", ir.QuoteIdentifier(constraint.Name), modifier, strings.Join(cols, ", ")) case ir.ConstraintTypeForeignKey: // Always include CONSTRAINT name to preserve explicit FK names // Use QualifyEntityNameWithQuotes to add schema qualifier when referencing tables in other schemas @@ -176,6 +180,9 @@ func constraintsEqual(old, new *ir.Constraint) bool { if old.IsTemporal != new.IsTemporal { return false } + if old.NullsNotDistinct != new.NullsNotDistinct { + return false + } // Validation status - only compare for CHECK and FOREIGN KEY constraints // PRIMARY KEY and UNIQUE constraints are always valid (IsValid is not meaningful for them) diff --git a/internal/diff/table.go b/internal/diff/table.go index 5f0dbb76..2d68a2c8 100644 --- a/internal/diff/table.go +++ b/internal/diff/table.go @@ -774,7 +774,11 @@ func (td *tableDiff) generateAlterTableStatements(targetSchema string, collector case ir.ConstraintTypePrimaryKey: inlineConstraint = fmt.Sprintf(" CONSTRAINT %s PRIMARY KEY", ir.QuoteIdentifier(constraint.Name)) case ir.ConstraintTypeUnique: - inlineConstraint = fmt.Sprintf(" CONSTRAINT %s UNIQUE", ir.QuoteIdentifier(constraint.Name)) + modifier := "" + if constraint.NullsNotDistinct { + modifier = " NULLS NOT DISTINCT" + } + inlineConstraint = fmt.Sprintf(" CONSTRAINT %s UNIQUE%s", ir.QuoteIdentifier(constraint.Name), modifier) case ir.ConstraintTypeForeignKey: // For FK, use the generateForeignKeyClause with inline=true fkClause := generateForeignKeyClause(constraint, targetSchema, true) @@ -876,8 +880,12 @@ func (td *tableDiff) generateAlterTableStatements(targetSchema string, collector columnNames[len(columnNames)-1] = columnNames[len(columnNames)-1] + " WITHOUT OVERLAPS" } tableName := getTableNameWithSchema(td.Table.Schema, td.Table.Name, targetSchema) - sql := fmt.Sprintf("ALTER TABLE %s\nADD CONSTRAINT %s UNIQUE (%s);", - tableName, ir.QuoteIdentifier(constraint.Name), strings.Join(columnNames, ", ")) + modifier := "" + if constraint.NullsNotDistinct { + modifier = " NULLS NOT DISTINCT" + } + sql := fmt.Sprintf("ALTER TABLE %s\nADD CONSTRAINT %s UNIQUE%s (%s);", + tableName, ir.QuoteIdentifier(constraint.Name), modifier, strings.Join(columnNames, ", ")) context := &diffContext{ Type: DiffTypeTableConstraint, @@ -1004,8 +1012,12 @@ func (td *tableDiff) generateAlterTableStatements(targetSchema string, collector if constraint.IsTemporal && len(columnNames) > 0 { columnNames[len(columnNames)-1] = columnNames[len(columnNames)-1] + " WITHOUT OVERLAPS" } - addSQL = fmt.Sprintf("ALTER TABLE %s\nADD CONSTRAINT %s UNIQUE (%s);", - tableName, ir.QuoteIdentifier(constraint.Name), strings.Join(columnNames, ", ")) + modifier := "" + if constraint.NullsNotDistinct { + modifier = " NULLS NOT DISTINCT" + } + addSQL = fmt.Sprintf("ALTER TABLE %s\nADD CONSTRAINT %s UNIQUE%s (%s);", + tableName, ir.QuoteIdentifier(constraint.Name), modifier, strings.Join(columnNames, ", ")) case ir.ConstraintTypeCheck: // Add CHECK constraint with ensured outer parentheses diff --git a/ir/inspector.go b/ir/inspector.go index 88397f09..a54821a4 100644 --- a/ir/inspector.go +++ b/ir/inspector.go @@ -486,13 +486,14 @@ func (i *Inspector) buildConstraints(ctx context.Context, schema *IR, targetSche } c = &Constraint{ - Schema: schemaName, - Table: tableName, - Name: constraintName, - Type: cType, - Columns: []*ConstraintColumn{}, - NoInherit: constraint.NoInherit, - IsTemporal: constraint.IsPeriod.Bool, // PG18 temporal constraint (WITHOUT OVERLAPS / PERIOD) + Schema: schemaName, + Table: tableName, + Name: constraintName, + Type: cType, + Columns: []*ConstraintColumn{}, + NoInherit: constraint.NoInherit, + IsTemporal: constraint.IsPeriod.Bool, // PG18 temporal constraint (WITHOUT OVERLAPS / PERIOD) + NullsNotDistinct: constraint.NullsNotDistinct.Bool, // PG15+ UNIQUE NULLS NOT DISTINCT } // Handle foreign key references diff --git a/ir/ir.go b/ir/ir.go index dd61bb8c..b73fdb49 100644 --- a/ir/ir.go +++ b/ir/ir.go @@ -222,8 +222,9 @@ type Constraint struct { Deferrable bool `json:"deferrable,omitempty"` InitiallyDeferred bool `json:"initially_deferred,omitempty"` IsValid bool `json:"is_valid,omitempty"` - NoInherit bool `json:"no_inherit,omitempty"` // CHECK constraint NO INHERIT modifier - IsTemporal bool `json:"is_temporal,omitempty"` // PG18: temporal constraint (WITHOUT OVERLAPS on PK/UNIQUE, PERIOD on FK) + NoInherit bool `json:"no_inherit,omitempty"` // CHECK constraint NO INHERIT modifier + IsTemporal bool `json:"is_temporal,omitempty"` // PG18: temporal constraint (WITHOUT OVERLAPS on PK/UNIQUE, PERIOD on FK) + NullsNotDistinct bool `json:"nulls_not_distinct,omitempty"` // PG15+: UNIQUE constraint treats NULLs as not distinct Comment string `json:"comment,omitempty"` } diff --git a/ir/queries/queries.sql b/ir/queries/queries.sql index d4ed8859..781a8b74 100644 --- a/ir/queries/queries.sql +++ b/ir/queries/queries.sql @@ -930,7 +930,10 @@ SELECT c.condeferred AS initially_deferred, c.convalidated AS is_valid, COALESCE((to_jsonb(c) ->> 'conperiod')::boolean, false) AS is_period, - c.connoinherit AS no_inherit + c.connoinherit AS no_inherit, + -- pg_index.indnullsnotdistinct is PG15+. Use to_jsonb so the column reference + -- doesn't fail to plan on PG14 (where the attribute does not exist on pg_index). + COALESCE((to_jsonb(i) ->> 'indnullsnotdistinct')::boolean, false) AS nulls_not_distinct FROM pg_constraint c JOIN pg_class cl ON c.conrelid = cl.oid JOIN pg_namespace n ON cl.relnamespace = n.oid @@ -938,6 +941,7 @@ LEFT JOIN pg_attribute a ON a.attrelid = c.conrelid AND a.attnum = ANY(c.conkey) LEFT JOIN pg_class fcl ON c.confrelid = fcl.oid LEFT JOIN pg_namespace fn ON fcl.relnamespace = fn.oid LEFT JOIN pg_attribute fa ON fa.attrelid = c.confrelid AND fa.attnum = c.confkey[array_position(c.conkey, a.attnum)] +LEFT JOIN pg_index i ON i.indexrelid = c.conindid WHERE n.nspname = $1 ORDER BY n.nspname, cl.relname, c.contype, c.conname, a.attnum; diff --git a/ir/queries/queries.sql.go b/ir/queries/queries.sql.go index 428617c6..911316ff 100644 --- a/ir/queries/queries.sql.go +++ b/ir/queries/queries.sql.go @@ -923,7 +923,10 @@ SELECT c.condeferred AS initially_deferred, c.convalidated AS is_valid, COALESCE((to_jsonb(c) ->> 'conperiod')::boolean, false) AS is_period, - c.connoinherit AS no_inherit + c.connoinherit AS no_inherit, + -- pg_index.indnullsnotdistinct is PG15+. Use to_jsonb so the column reference + -- doesn't fail to plan on PG14 (where the attribute does not exist on pg_index). + COALESCE((to_jsonb(i) ->> 'indnullsnotdistinct')::boolean, false) AS nulls_not_distinct FROM pg_constraint c JOIN pg_class cl ON c.conrelid = cl.oid JOIN pg_namespace n ON cl.relnamespace = n.oid @@ -931,6 +934,7 @@ LEFT JOIN pg_attribute a ON a.attrelid = c.conrelid AND a.attnum = ANY(c.conkey) LEFT JOIN pg_class fcl ON c.confrelid = fcl.oid LEFT JOIN pg_namespace fn ON fcl.relnamespace = fn.oid LEFT JOIN pg_attribute fa ON fa.attrelid = c.confrelid AND fa.attnum = c.confkey[array_position(c.conkey, a.attnum)] +LEFT JOIN pg_index i ON i.indexrelid = c.conindid WHERE n.nspname = $1 ORDER BY n.nspname, cl.relname, c.contype, c.conname, a.attnum ` @@ -955,6 +959,7 @@ type GetConstraintsForSchemaRow struct { IsValid bool `db:"is_valid" json:"is_valid"` IsPeriod sql.NullBool `db:"is_period" json:"is_period"` NoInherit bool `db:"no_inherit" json:"no_inherit"` + NullsNotDistinct sql.NullBool `db:"nulls_not_distinct" json:"nulls_not_distinct"` } // GetConstraintsForSchema retrieves all table constraints for a specific schema @@ -987,6 +992,7 @@ func (q *Queries) GetConstraintsForSchema(ctx context.Context, dollar_1 sql.Null &i.IsValid, &i.IsPeriod, &i.NoInherit, + &i.NullsNotDistinct, ); err != nil { return nil, err } diff --git a/testdata/diff/create_table/add_unique_constraint_nulls_not_distinct/diff.sql b/testdata/diff/create_table/add_unique_constraint_nulls_not_distinct/diff.sql new file mode 100644 index 00000000..5ab239fc --- /dev/null +++ b/testdata/diff/create_table/add_unique_constraint_nulls_not_distinct/diff.sql @@ -0,0 +1,2 @@ +ALTER TABLE pgschema_repro_nulls +ADD CONSTRAINT pgschema_repro_nulls_uniq UNIQUE NULLS NOT DISTINCT (a, b); diff --git a/testdata/diff/create_table/add_unique_constraint_nulls_not_distinct/new.sql b/testdata/diff/create_table/add_unique_constraint_nulls_not_distinct/new.sql new file mode 100644 index 00000000..4572bafa --- /dev/null +++ b/testdata/diff/create_table/add_unique_constraint_nulls_not_distinct/new.sql @@ -0,0 +1,11 @@ +-- Regression for: UNIQUE NULLS NOT DISTINCT modifier dropped from constraints +-- Add a table-level UNIQUE constraint with the NULLS NOT DISTINCT modifier +-- (PostgreSQL 15+). Without the fix, the inspector loses the modifier and the +-- generated migration emits a plain UNIQUE (a, b) — silently changing the +-- semantics of the constraint and breaking INSERT ... ON CONFLICT flows that +-- rely on NULLs colliding. +CREATE TABLE public.pgschema_repro_nulls ( + a integer, + b integer, + CONSTRAINT pgschema_repro_nulls_uniq UNIQUE NULLS NOT DISTINCT (a, b) +); diff --git a/testdata/diff/create_table/add_unique_constraint_nulls_not_distinct/old.sql b/testdata/diff/create_table/add_unique_constraint_nulls_not_distinct/old.sql new file mode 100644 index 00000000..f9d7e1fb --- /dev/null +++ b/testdata/diff/create_table/add_unique_constraint_nulls_not_distinct/old.sql @@ -0,0 +1,6 @@ +-- Regression for: UNIQUE NULLS NOT DISTINCT modifier dropped from constraints +-- The starting state has no UNIQUE constraint at all; we add one in new.sql. +CREATE TABLE public.pgschema_repro_nulls ( + a integer, + b integer +); diff --git a/testdata/diff/create_table/add_unique_constraint_nulls_not_distinct/plan.json b/testdata/diff/create_table/add_unique_constraint_nulls_not_distinct/plan.json new file mode 100644 index 00000000..726fb0e3 --- /dev/null +++ b/testdata/diff/create_table/add_unique_constraint_nulls_not_distinct/plan.json @@ -0,0 +1,20 @@ +{ + "version": "1.0.0", + "pgschema_version": "1.9.0", + "created_at": "1970-01-01T00:00:00Z", + "source_fingerprint": { + "hash": "70a4465367d5d40d0149eadc73c423f9eb954838b6602f00ca3496b264baf2e9" + }, + "groups": [ + { + "steps": [ + { + "sql": "ALTER TABLE pgschema_repro_nulls\nADD CONSTRAINT pgschema_repro_nulls_uniq UNIQUE NULLS NOT DISTINCT (a, b);", + "type": "table.constraint", + "operation": "create", + "path": "public.pgschema_repro_nulls.pgschema_repro_nulls_uniq" + } + ] + } + ] +} diff --git a/testdata/diff/create_table/add_unique_constraint_nulls_not_distinct/plan.sql b/testdata/diff/create_table/add_unique_constraint_nulls_not_distinct/plan.sql new file mode 100644 index 00000000..5ab239fc --- /dev/null +++ b/testdata/diff/create_table/add_unique_constraint_nulls_not_distinct/plan.sql @@ -0,0 +1,2 @@ +ALTER TABLE pgschema_repro_nulls +ADD CONSTRAINT pgschema_repro_nulls_uniq UNIQUE NULLS NOT DISTINCT (a, b); diff --git a/testdata/diff/create_table/add_unique_constraint_nulls_not_distinct/plan.txt b/testdata/diff/create_table/add_unique_constraint_nulls_not_distinct/plan.txt new file mode 100644 index 00000000..cc5a1d0f --- /dev/null +++ b/testdata/diff/create_table/add_unique_constraint_nulls_not_distinct/plan.txt @@ -0,0 +1,14 @@ +Plan: 1 to modify. + +Summary by type: + tables: 1 to modify + +Tables: + ~ pgschema_repro_nulls + + pgschema_repro_nulls_uniq (constraint) + +DDL to be executed: +-------------------------------------------------- + +ALTER TABLE pgschema_repro_nulls +ADD CONSTRAINT pgschema_repro_nulls_uniq UNIQUE NULLS NOT DISTINCT (a, b); diff --git a/testdata/dump/issue_412_unique_nulls_not_distinct/manifest.json b/testdata/dump/issue_412_unique_nulls_not_distinct/manifest.json new file mode 100644 index 00000000..c57bcd6b --- /dev/null +++ b/testdata/dump/issue_412_unique_nulls_not_distinct/manifest.json @@ -0,0 +1,9 @@ +{ + "name": "issue_412_unique_nulls_not_distinct", + "description": "pgschema dump silently drops the NULLS NOT DISTINCT modifier from UNIQUE table constraints", + "source": "https://github.com/pgplex/pgschema/issues/412", + "notes": [ + "NULLS NOT DISTINCT was introduced in PostgreSQL 15; the underlying pg_index.indnullsnotdistinct column does not exist on PG14.", + "The inspector exposes the modifier on UNIQUE constraints by joining pg_index via conindid and reading indnullsnotdistinct defensively (to_jsonb), which collapses to false on PG14." + ] +} diff --git a/testdata/dump/issue_412_unique_nulls_not_distinct/pgdump.sql b/testdata/dump/issue_412_unique_nulls_not_distinct/pgdump.sql new file mode 100644 index 00000000..5c84403e --- /dev/null +++ b/testdata/dump/issue_412_unique_nulls_not_distinct/pgdump.sql @@ -0,0 +1,32 @@ +-- +-- PostgreSQL database dump +-- + +SET statement_timeout = 0; +SET lock_timeout = 0; +-- SET transaction_timeout = 0; +SET client_encoding = 'UTF8'; +SET standard_conforming_strings = on; +SET check_function_bodies = false; +SET client_min_messages = warning; +SET row_security = off; + +-- +-- Name: pgschema_repro_nulls; Type: TABLE; Schema: public; Owner: - +-- + +CREATE TABLE public.pgschema_repro_nulls ( + a integer, + b integer +); + +-- +-- Name: pgschema_repro_nulls pgschema_repro_nulls_uniq; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.pgschema_repro_nulls + ADD CONSTRAINT pgschema_repro_nulls_uniq UNIQUE NULLS NOT DISTINCT (a, b); + +-- +-- PostgreSQL database dump complete +-- diff --git a/testdata/dump/issue_412_unique_nulls_not_distinct/pgschema.sql b/testdata/dump/issue_412_unique_nulls_not_distinct/pgschema.sql new file mode 100644 index 00000000..c5f3b02f --- /dev/null +++ b/testdata/dump/issue_412_unique_nulls_not_distinct/pgschema.sql @@ -0,0 +1,18 @@ +-- +-- pgschema database dump +-- + +-- Dumped from database version PostgreSQL 18.0 +-- Dumped by pgschema version 1.9.0 + + +-- +-- Name: pgschema_repro_nulls; Type: TABLE; Schema: -; Owner: - +-- + +CREATE TABLE IF NOT EXISTS pgschema_repro_nulls ( + a integer, + b integer, + CONSTRAINT pgschema_repro_nulls_uniq UNIQUE NULLS NOT DISTINCT (a, b) +); + diff --git a/testdata/dump/issue_412_unique_nulls_not_distinct/raw.sql b/testdata/dump/issue_412_unique_nulls_not_distinct/raw.sql new file mode 100644 index 00000000..8e5b1945 --- /dev/null +++ b/testdata/dump/issue_412_unique_nulls_not_distinct/raw.sql @@ -0,0 +1,14 @@ +-- +-- Test case for GitHub issue #412: UNIQUE NULLS NOT DISTINCT dropped from dump +-- +-- The NULLS NOT DISTINCT modifier (PostgreSQL 15+) makes NULL-bearing tuples +-- collide for uniqueness purposes, which is the opposite of the SQL default. +-- pgschema dump used to silently drop the modifier, emitting a plain +-- UNIQUE (...) constraint and quietly changing semantics. +-- + +CREATE TABLE pgschema_repro_nulls ( + a integer, + b integer, + CONSTRAINT pgschema_repro_nulls_uniq UNIQUE NULLS NOT DISTINCT (a, b) +); diff --git a/testutil/skip_list.go b/testutil/skip_list.go index 39462a68..ed7cae8e 100644 --- a/testutil/skip_list.go +++ b/testutil/skip_list.go @@ -68,6 +68,8 @@ var skipListRequiresExtension = []string{ // These tests use features not available in PostgreSQL 14 (e.g., NULLS NOT DISTINCT is PG15+). var skipListPG14 = []string{ "create_index/add_index", + "create_table/add_unique_constraint_nulls_not_distinct", + "TestDumpCommand_Issue412UniqueNullsNotDistinct", } // skipListPG14_17 defines test cases that should be skipped for PostgreSQL 14-17.