Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cmd/dump/dump_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,11 @@ func TestDumpCommand_TenantSchemas(t *testing.T) {
runTenantSchemaTest(t, "tenant")
}

func TestDumpCommand_Issue78ConstraintNotValid(t *testing.T) {
func TestDumpCommand_Issue78ConstraintNotValidAndQuoting(t *testing.T) {
if testing.Short() {
t.Skip("Skipping integration test in short mode")
}
runExactMatchTest(t, "issue_78_constraint_not_valid")
runExactMatchTest(t, "issue_78_constraint_not_valid_and_quoting")
}

func TestDumpCommand_Issue80IndexNameQuote(t *testing.T) {
Expand Down
8 changes: 4 additions & 4 deletions internal/diff/constraint.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,16 @@ func generateConstraintSQL(constraint *ir.Constraint, targetSchema string) strin
switch constraint.Type {
case ir.ConstraintTypePrimaryKey:
// Always include CONSTRAINT name to be explicit and consistent
return fmt.Sprintf("CONSTRAINT %s PRIMARY KEY (%s)", constraint.Name, strings.Join(getColumnNames(constraint.Columns), ", "))
return fmt.Sprintf("CONSTRAINT %s PRIMARY KEY (%s)", ir.QuoteIdentifier(constraint.Name), strings.Join(getColumnNames(constraint.Columns), ", "))
case ir.ConstraintTypeUnique:
// Always include CONSTRAINT name to be explicit and consistent
return fmt.Sprintf("CONSTRAINT %s UNIQUE (%s)", constraint.Name, strings.Join(getColumnNames(constraint.Columns), ", "))
return fmt.Sprintf("CONSTRAINT %s UNIQUE (%s)", ir.QuoteIdentifier(constraint.Name), strings.Join(getColumnNames(constraint.Columns), ", "))
case ir.ConstraintTypeForeignKey:
// Always include CONSTRAINT name to preserve explicit FK names
// Use QualifyEntityNameWithQuotes to add schema qualifier when referencing tables in other schemas
qualifiedRefTable := ir.QualifyEntityNameWithQuotes(constraint.ReferencedSchema, constraint.ReferencedTable, targetSchema)
stmt := fmt.Sprintf("CONSTRAINT %s FOREIGN KEY (%s) REFERENCES %s (%s)",
constraint.Name,
ir.QuoteIdentifier(constraint.Name),
strings.Join(getColumnNames(constraint.Columns), ", "),
qualifiedRefTable, strings.Join(getColumnNames(constraint.ReferencedColumns), ", "))
// Only add ON UPDATE/DELETE if they are not the default "NO ACTION"
Expand All @@ -57,7 +57,7 @@ func generateConstraintSQL(constraint *ir.Constraint, targetSchema string) strin
// Generate CHECK constraint with proper NOT VALID placement
// The CheckClause is normalized to exclude NOT VALID (stripped in normalize.go)
// We append NOT VALID based on IsValid field, mimicking pg_dump behavior
result := fmt.Sprintf("CONSTRAINT %s %s", constraint.Name, constraint.CheckClause)
result := fmt.Sprintf("CONSTRAINT %s %s", ir.QuoteIdentifier(constraint.Name), constraint.CheckClause)
if !constraint.IsValid {
result += " NOT VALID"
}
Expand Down
28 changes: 14 additions & 14 deletions internal/diff/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,7 @@ func (td *tableDiff) generateAlterTableStatements(targetSchema string, collector
// Drop constraints first (before dropping columns) - already sorted by the Diff operation
for _, constraint := range td.DroppedConstraints {
tableName := getTableNameWithSchema(td.Table.Schema, td.Table.Name, targetSchema)
sql := fmt.Sprintf("ALTER TABLE %s DROP CONSTRAINT %s;", tableName, constraint.Name)
sql := fmt.Sprintf("ALTER TABLE %s DROP CONSTRAINT %s;", tableName, ir.QuoteIdentifier(constraint.Name))

context := &diffContext{
Type: DiffTypeTableConstraint,
Expand Down Expand Up @@ -663,13 +663,13 @@ func (td *tableDiff) generateAlterTableStatements(targetSchema string, collector
if len(constraint.Columns) == 1 && constraint.Columns[0].Name == column.Name {
switch constraint.Type {
case ir.ConstraintTypePrimaryKey:
inlineConstraint = fmt.Sprintf(" CONSTRAINT %s PRIMARY KEY", constraint.Name)
inlineConstraint = fmt.Sprintf(" CONSTRAINT %s PRIMARY KEY", ir.QuoteIdentifier(constraint.Name))
case ir.ConstraintTypeUnique:
inlineConstraint = fmt.Sprintf(" CONSTRAINT %s UNIQUE", constraint.Name)
inlineConstraint = fmt.Sprintf(" CONSTRAINT %s UNIQUE", ir.QuoteIdentifier(constraint.Name))
case ir.ConstraintTypeForeignKey:
// For FK, use the generateForeignKeyClause with inline=true
fkClause := generateForeignKeyClause(constraint, targetSchema, true)
inlineConstraint = fmt.Sprintf(" CONSTRAINT %s%s", constraint.Name, fkClause)
inlineConstraint = fmt.Sprintf(" CONSTRAINT %s%s", ir.QuoteIdentifier(constraint.Name), fkClause)
case ir.ConstraintTypeCheck:
// For CHECK, format the clause inline
checkExpr := constraint.CheckClause
Expand All @@ -682,7 +682,7 @@ func (td *tableDiff) generateAlterTableStatements(targetSchema string, collector
if !strings.HasPrefix(checkExpr, "(") {
checkExpr = "(" + checkExpr + ")"
}
inlineConstraint = fmt.Sprintf(" CONSTRAINT %s CHECK %s", constraint.Name, checkExpr)
inlineConstraint = fmt.Sprintf(" CONSTRAINT %s CHECK %s", ir.QuoteIdentifier(constraint.Name), checkExpr)
}

if inlineConstraint != "" {
Expand Down Expand Up @@ -748,7 +748,7 @@ func (td *tableDiff) generateAlterTableStatements(targetSchema string, collector
}
tableName := getTableNameWithSchema(td.Table.Schema, td.Table.Name, targetSchema)
sql := fmt.Sprintf("ALTER TABLE %s\nADD CONSTRAINT %s UNIQUE (%s);",
tableName, constraint.Name, strings.Join(columnNames, ", "))
tableName, ir.QuoteIdentifier(constraint.Name), strings.Join(columnNames, ", "))

context := &diffContext{
Type: DiffTypeTableConstraint,
Expand All @@ -764,7 +764,7 @@ func (td *tableDiff) generateAlterTableStatements(targetSchema string, collector
tableName := getTableNameWithSchema(td.Table.Schema, td.Table.Name, targetSchema)
clause := ensureCheckClauseParens(constraint.CheckClause)
canonicalSQL := fmt.Sprintf("ALTER TABLE %s\nADD CONSTRAINT %s %s;",
tableName, constraint.Name, clause)
tableName, ir.QuoteIdentifier(constraint.Name), clause)

context := &diffContext{
Type: DiffTypeTableConstraint,
Expand All @@ -785,7 +785,7 @@ func (td *tableDiff) generateAlterTableStatements(targetSchema string, collector

tableName := getTableNameWithSchema(td.Table.Schema, td.Table.Name, targetSchema)
canonicalSQL := fmt.Sprintf("ALTER TABLE %s\nADD CONSTRAINT %s FOREIGN KEY (%s) %s;",
tableName, constraint.Name,
tableName, ir.QuoteIdentifier(constraint.Name),
strings.Join(columnNames, ", "),
generateForeignKeyClause(constraint, targetSchema, false))

Expand All @@ -807,7 +807,7 @@ func (td *tableDiff) generateAlterTableStatements(targetSchema string, collector
}
tableName := getTableNameWithSchema(td.Table.Schema, td.Table.Name, targetSchema)
sql := fmt.Sprintf("ALTER TABLE %s\nADD CONSTRAINT %s PRIMARY KEY (%s);",
tableName, constraint.Name, strings.Join(columnNames, ", "))
tableName, ir.QuoteIdentifier(constraint.Name), strings.Join(columnNames, ", "))

context := &diffContext{
Type: DiffTypeTableConstraint,
Expand All @@ -826,7 +826,7 @@ func (td *tableDiff) generateAlterTableStatements(targetSchema string, collector
constraint := ConstraintDiff.New

// Step 1: Drop the old constraint
dropSQL := fmt.Sprintf("ALTER TABLE %s DROP CONSTRAINT %s;", tableName, ConstraintDiff.Old.Name)
dropSQL := fmt.Sprintf("ALTER TABLE %s DROP CONSTRAINT %s;", tableName, ir.QuoteIdentifier(ConstraintDiff.Old.Name))
dropContext := &diffContext{
Type: DiffTypeTableConstraint,
Operation: DiffOperationDrop,
Expand All @@ -847,12 +847,12 @@ func (td *tableDiff) generateAlterTableStatements(targetSchema string, collector
columnNames = append(columnNames, ir.QuoteIdentifier(col.Name))
}
addSQL = fmt.Sprintf("ALTER TABLE %s\nADD CONSTRAINT %s UNIQUE (%s);",
tableName, constraint.Name, strings.Join(columnNames, ", "))
tableName, ir.QuoteIdentifier(constraint.Name), strings.Join(columnNames, ", "))

case ir.ConstraintTypeCheck:
// Add CHECK constraint with ensured outer parentheses
addSQL = fmt.Sprintf("ALTER TABLE %s\nADD CONSTRAINT %s %s;",
tableName, constraint.Name, ensureCheckClauseParens(constraint.CheckClause))
tableName, ir.QuoteIdentifier(constraint.Name), ensureCheckClauseParens(constraint.CheckClause))

case ir.ConstraintTypeForeignKey:
// Sort columns by position
Expand All @@ -863,7 +863,7 @@ func (td *tableDiff) generateAlterTableStatements(targetSchema string, collector
}

addSQL = fmt.Sprintf("ALTER TABLE %s\nADD CONSTRAINT %s FOREIGN KEY (%s) %s;",
tableName, constraint.Name,
tableName, ir.QuoteIdentifier(constraint.Name),
strings.Join(columnNames, ", "),
generateForeignKeyClause(constraint, targetSchema, false))

Expand All @@ -875,7 +875,7 @@ func (td *tableDiff) generateAlterTableStatements(targetSchema string, collector
columnNames = append(columnNames, ir.QuoteIdentifier(col.Name))
}
addSQL = fmt.Sprintf("ALTER TABLE %s\nADD CONSTRAINT %s PRIMARY KEY (%s);",
tableName, constraint.Name, strings.Join(columnNames, ", "))
tableName, ir.QuoteIdentifier(constraint.Name), strings.Join(columnNames, ", "))
}

addContext := &diffContext{
Expand Down
2 changes: 1 addition & 1 deletion internal/diff/trigger.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ func generateTriggerSQLWithMode(trigger *ir.Trigger, targetSchema string) string
var stmt string
if trigger.IsConstraint {
stmt = fmt.Sprintf("CREATE CONSTRAINT TRIGGER %s\n %s %s ON %s",
trigger.Name, trigger.Timing, eventList, tableName)
ir.QuoteIdentifier(trigger.Name), trigger.Timing, eventList, tableName)

// Add deferrable clause for constraint triggers
if trigger.Deferrable {
Expand Down
8 changes: 4 additions & 4 deletions internal/diff/type.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,12 +213,12 @@ func generateAlterDomainStatements(oldDomain, newDomain *ir.Type, targetSchema s
if newConstraint, exists := newConstraints[key]; !exists {
// Constraint was removed
if oldConstraint.Name != "" {
statements = append(statements, fmt.Sprintf("ALTER DOMAIN %s DROP CONSTRAINT %s;", domainName, oldConstraint.Name))
statements = append(statements, fmt.Sprintf("ALTER DOMAIN %s DROP CONSTRAINT %s;", domainName, ir.QuoteIdentifier(oldConstraint.Name)))
}
// Note: unnamed constraints cannot be dropped individually
} else if oldConstraint.Name != "" && oldConstraint.Definition != newConstraint.Definition {
// Constraint exists but definition changed - need to drop and recreate
statements = append(statements, fmt.Sprintf("ALTER DOMAIN %s DROP CONSTRAINT %s;", domainName, oldConstraint.Name))
statements = append(statements, fmt.Sprintf("ALTER DOMAIN %s DROP CONSTRAINT %s;", domainName, ir.QuoteIdentifier(oldConstraint.Name)))
}
}

Expand All @@ -229,7 +229,7 @@ func generateAlterDomainStatements(oldDomain, newDomain *ir.Type, targetSchema s
// Either new constraint or definition changed
constraintDef := newConstraint.Definition
if newConstraint.Name != "" {
statements = append(statements, fmt.Sprintf("ALTER DOMAIN %s ADD CONSTRAINT %s %s;", domainName, newConstraint.Name, constraintDef))
statements = append(statements, fmt.Sprintf("ALTER DOMAIN %s ADD CONSTRAINT %s %s;", domainName, ir.QuoteIdentifier(newConstraint.Name), constraintDef))
} else {
statements = append(statements, fmt.Sprintf("ALTER DOMAIN %s ADD %s;", domainName, constraintDef))
}
Expand Down Expand Up @@ -295,7 +295,7 @@ func generateTypeSQL(typeObj *ir.Type, targetSchema string) string {
for _, constraint := range typeObj.Constraints {
constraintDef := constraint.Definition
if constraint.Name != "" {
lines = append(lines, fmt.Sprintf(" CONSTRAINT %s %s", constraint.Name, constraintDef))
lines = append(lines, fmt.Sprintf(" CONSTRAINT %s %s", ir.QuoteIdentifier(constraint.Name), constraintDef))
} else {
lines = append(lines, fmt.Sprintf(" %s", constraintDef))
}
Expand Down
12 changes: 6 additions & 6 deletions internal/plan/rewrite.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,9 +194,9 @@ func generateConstraintRewrite(constraint *ir.Constraint) []RewriteStep {
tableName := getTableNameWithSchema(constraint.Schema, constraint.Table)

notValidSQL := fmt.Sprintf("ALTER TABLE %s\nADD CONSTRAINT %s %s NOT VALID;",
tableName, constraint.Name, constraint.CheckClause)
tableName, ir.QuoteIdentifier(constraint.Name), constraint.CheckClause)
validateSQL := fmt.Sprintf("ALTER TABLE %s VALIDATE CONSTRAINT %s;",
tableName, constraint.Name)
tableName, ir.QuoteIdentifier(constraint.Name))

return []RewriteStep{
{
Expand Down Expand Up @@ -249,9 +249,9 @@ func generateForeignKeyRewrite(constraint *ir.Constraint) []RewriteStep {
}

notValidSQL := fmt.Sprintf("ALTER TABLE %s\nADD CONSTRAINT %s %s NOT VALID;",
tableName, constraint.Name, fkClause)
tableName, ir.QuoteIdentifier(constraint.Name), fkClause)
validateSQL := fmt.Sprintf("ALTER TABLE %s VALIDATE CONSTRAINT %s;",
tableName, constraint.Name)
tableName, ir.QuoteIdentifier(constraint.Name))

return []RewriteStep{
{
Expand Down Expand Up @@ -283,11 +283,11 @@ func generateColumnNotNullRewrite(_ *diff.ColumnDiff, path string) []RewriteStep

// Step 1: Add CHECK constraint with NOT VALID
addConstraintSQL := fmt.Sprintf("ALTER TABLE %s ADD CONSTRAINT %s CHECK (%s IS NOT NULL) NOT VALID;",
tableName, constraintName, column)
tableName, ir.QuoteIdentifier(constraintName), column)

// Step 2: Validate the constraint
validateConstraintSQL := fmt.Sprintf("ALTER TABLE %s VALIDATE CONSTRAINT %s;",
tableName, constraintName)
tableName, ir.QuoteIdentifier(constraintName))

// Step 3: Set column to NOT NULL
setNotNullSQL := fmt.Sprintf("ALTER TABLE %s ALTER COLUMN %s SET NOT NULL;",
Expand Down
8 changes: 0 additions & 8 deletions testdata/dump/issue_78_constraint_not_valid/manifest.json

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"name": "issue_78_constraint_not_valid_and_quoting",
"description": "Test case for NOT VALID constraint preservation and constraint name quoting (GitHub issue #78)",
"source": "https://github.com/pgschema/pgschema/issues/78",
"notes": [
"Tests that NOT VALID constraints are correctly preserved when dumping",
"Tests that constraint names with whitespace are properly quoted (e.g., 'price not negative')",
"Verifies constraint name quoting handles identifiers that need quotes per PostgreSQL rules"
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,14 @@ SET default_tablespace = '';
SET default_table_access_method = heap;

--
-- Name: products; Type: TABLE; Schema: public; Owner: postgres
-- Name: products; Type: TABLE; Schema: public; Owner: -
--

CREATE TABLE public.products (
id uuid DEFAULT gen_random_uuid() NOT NULL,
name text NOT NULL,
price numeric(10,2) NOT NULL,
CONSTRAINT "price not negative" CHECK ((price >= (0)::numeric)),
CONSTRAINT products_price_positive CHECK ((price > (0)::numeric))
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ CREATE TABLE IF NOT EXISTS products (
id uuid DEFAULT gen_random_uuid() NOT NULL,
name text NOT NULL,
price numeric(10,2) NOT NULL,
CONSTRAINT "price not negative" CHECK (price >= 0::numeric),
CONSTRAINT products_price_positive CHECK (price > 0::numeric)
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ CREATE TABLE products (
ALTER TABLE products
ADD CONSTRAINT products_price_positive CHECK (price > 0);

-- Add constraint with whitespace in name to test proper quoting
-- Tests GitHub issue #78 comment about constraint names with spaces
ALTER TABLE products
ADD CONSTRAINT "price not negative" CHECK (price >= 0);


--
-- Case 2: Table WITH data - constraint added with NOT VALID
Expand Down
Loading