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
7 changes: 7 additions & 0 deletions cmd/dump/dump_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,13 @@ func TestDumpCommand_Issue80IndexNameQuote(t *testing.T) {
runExactMatchTest(t, "issue_80_index_name_quote")
}

func TestDumpCommand_Issue83ExplicitConstraintName(t *testing.T) {
if testing.Short() {
t.Skip("Skipping integration test in short mode")
}
runExactMatchTest(t, "issue_83_explicit_constraint_name")
}

func runExactMatchTest(t *testing.T, testDataDir string) {
runExactMatchTestWithContext(t, context.Background(), testDataDir)
}
Expand Down
44 changes: 24 additions & 20 deletions internal/diff/constraint.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,15 @@ func generateConstraintSQL(constraint *ir.Constraint, _ string) string {

switch constraint.Type {
case ir.ConstraintTypePrimaryKey:
return fmt.Sprintf("PRIMARY KEY (%s)", strings.Join(getColumnNames(constraint.Columns), ", "))
// Always include CONSTRAINT name to be explicit and consistent
return fmt.Sprintf("CONSTRAINT %s PRIMARY KEY (%s)", constraint.Name, strings.Join(getColumnNames(constraint.Columns), ", "))
case ir.ConstraintTypeUnique:
return fmt.Sprintf("UNIQUE (%s)", strings.Join(getColumnNames(constraint.Columns), ", "))
// Always include CONSTRAINT name to be explicit and consistent
return fmt.Sprintf("CONSTRAINT %s UNIQUE (%s)", constraint.Name, strings.Join(getColumnNames(constraint.Columns), ", "))
case ir.ConstraintTypeForeignKey:
stmt := fmt.Sprintf("FOREIGN KEY (%s) REFERENCES %s (%s)",
// Always include CONSTRAINT name to preserve explicit FK names
stmt := fmt.Sprintf("CONSTRAINT %s FOREIGN KEY (%s) REFERENCES %s (%s)",
constraint.Name,
strings.Join(getColumnNames(constraint.Columns), ", "),
ir.QuoteIdentifier(constraint.ReferencedTable), strings.Join(getColumnNames(constraint.ReferencedColumns), ", "))
// Only add ON UPDATE/DELETE if they are not the default "NO ACTION"
Expand All @@ -34,6 +38,18 @@ func generateConstraintSQL(constraint *ir.Constraint, _ string) string {
if constraint.DeleteRule != "" && constraint.DeleteRule != "NO ACTION" {
stmt += fmt.Sprintf(" ON DELETE %s", constraint.DeleteRule)
}
// Add deferrable clause
if constraint.Deferrable {
if constraint.InitiallyDeferred {
stmt += " DEFERRABLE INITIALLY DEFERRED"
} else {
stmt += " DEFERRABLE"
}
}
// Add NOT VALID if needed
if !constraint.IsValid {
stmt += " NOT VALID"
}
return stmt
case ir.ConstraintTypeCheck:
// Generate CHECK constraint with proper NOT VALID placement
Expand Down Expand Up @@ -66,28 +82,16 @@ func getInlineConstraintsForTable(table *ir.Table) []*ir.Constraint {
constraint := table.Constraints[constraintName]

// Categorize constraints by type
// ALL constraints are now included as table-level constraints for consistency
// This ensures all constraint names are preserved and provides cleaner formatting
switch constraint.Type {
case ir.ConstraintTypePrimaryKey:
// Only include multi-column primary keys as inline constraints
// Single-column primary keys are handled inline with the column definition
if len(constraint.Columns) > 1 {
primaryKeys = append(primaryKeys, constraint)
}
primaryKeys = append(primaryKeys, constraint)
case ir.ConstraintTypeUnique:
// Only include multi-column unique constraints as inline constraints
// Single-column unique constraints are handled inline with the column definition
if len(constraint.Columns) > 1 {
uniques = append(uniques, constraint)
}
uniques = append(uniques, constraint)
case ir.ConstraintTypeForeignKey:
// Only include multi-column foreign key constraints as inline constraints
// Single-column foreign key constraints are handled inline with the column definition
if len(constraint.Columns) > 1 {
foreignKeys = append(foreignKeys, constraint)
}
foreignKeys = append(foreignKeys, constraint)
case ir.ConstraintTypeCheck:
// Always include ALL CHECK constraints as table-level named constraints
// This eliminates complexity and makes constraints explicit and manageable
checkConstraints = append(checkConstraints, constraint)
}
}
Expand Down
9 changes: 5 additions & 4 deletions internal/diff/identifier_quote_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func TestGenerateConstraintSQL_WithQuoting(t *testing.T) {
{Name: "accountId", Position: 2},
},
},
want: `UNIQUE ("userId", "accountId")`,
want: `CONSTRAINT test_unique UNIQUE ("userId", "accountId")`,
},
{
name: "PRIMARY KEY with reserved word",
Expand All @@ -34,7 +34,7 @@ func TestGenerateConstraintSQL_WithQuoting(t *testing.T) {
{Name: "order", Position: 2},
},
},
want: `PRIMARY KEY ("user", "order")`,
want: `CONSTRAINT test_pk PRIMARY KEY ("user", "order")`,
},
{
name: "FOREIGN KEY with camelCase",
Expand All @@ -49,8 +49,9 @@ func TestGenerateConstraintSQL_WithQuoting(t *testing.T) {
{Name: "id", Position: 1},
},
DeleteRule: "CASCADE",
IsValid: true,
},
want: `FOREIGN KEY ("userId") REFERENCES users (id) ON DELETE CASCADE`,
want: `CONSTRAINT test_fk FOREIGN KEY ("userId") REFERENCES users (id) ON DELETE CASCADE`,
},
{
name: "UNIQUE with lowercase columns (no quotes needed)",
Expand All @@ -62,7 +63,7 @@ func TestGenerateConstraintSQL_WithQuoting(t *testing.T) {
{Name: "username", Position: 2},
},
},
want: `UNIQUE (email, username)`,
want: `CONSTRAINT test_unique_lower UNIQUE (email, username)`,
},
}

Expand Down
202 changes: 81 additions & 121 deletions internal/diff/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -523,54 +523,81 @@ func (td *tableDiff) generateAlterTableStatements(targetSchema string, collector
collector.collect(context, sql)
}

// Add new columns - already sorted by the Diff operation
// Track which constraints are handled inline with column additions
handledFKConstraints := make(map[string]bool)
handledPKConstraints := make(map[string]bool)
handledUKConstraints := make(map[string]bool)
// Track constraints that are added inline with columns to avoid duplicate generation
inlineConstraints := make(map[string]bool)

// Add new columns - already sorted by the Diff operation
for _, column := range td.AddedColumns {
// Convert table.Constraints map to slice for helper
var allConstraints []*ir.Constraint
for _, constraint := range td.Table.Constraints {
allConstraints = append(allConstraints, constraint)
}

// Find all single-column constraints for this column
// For ALTER TABLE, FK/CHECK come from table.Constraints, but PK/UK come from AddedConstraints
constraints, isPartOfAnyPK := findSingleColumnConstraints(column, allConstraints, td.AddedConstraints)

// Mark constraints as handled so we don't add them again later
if constraints.ForeignKey != nil {
handledFKConstraints[constraints.ForeignKey.Name] = true
}
if constraints.PrimaryKey != nil {
handledPKConstraints[constraints.PrimaryKey.Name] = true
}
if constraints.Unique != nil {
handledUKConstraints[constraints.Unique.Name] = true
// Check if column is part of any primary key constraint for NOT NULL handling
isPartOfAnyPK := false
for _, constraint := range td.AddedConstraints {
if constraint.Type == ir.ConstraintTypePrimaryKey {
for _, col := range constraint.Columns {
if col.Name == column.Name {
isPartOfAnyPK = true
break
}
}
if isPartOfAnyPK {
break
}
}
}

// Build column type
columnType := formatColumnDataType(column)
tableName := getTableNameWithSchema(td.Table.Schema, td.Table.Name, targetSchema)

// Use line break format for complex statements (with foreign keys, primary keys, or unique keys)
// Build and append all column clauses
clauses := buildColumnClauses(column, isPartOfAnyPK, td.Table.Schema, targetSchema)

// Check for single-column constraints that can be added inline
var inlineConstraint string
for _, constraint := range td.AddedConstraints {
// Only add inline for single-column constraints
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)
case ir.ConstraintTypeUnique:
inlineConstraint = fmt.Sprintf(" CONSTRAINT %s UNIQUE", 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)
case ir.ConstraintTypeCheck:
// For CHECK, format the clause inline
checkExpr := constraint.CheckClause
// Strip "CHECK" prefix if present
if len(checkExpr) >= 5 && strings.EqualFold(checkExpr[:5], "check") {
checkExpr = strings.TrimSpace(checkExpr[5:])
}
checkExpr = strings.TrimSpace(checkExpr)
// Ensure parentheses
if !strings.HasPrefix(checkExpr, "(") {
checkExpr = "(" + checkExpr + ")"
}
inlineConstraint = fmt.Sprintf(" CONSTRAINT %s CHECK %s", constraint.Name, checkExpr)
}

if inlineConstraint != "" {
inlineConstraints[constraint.Name] = true
break
}
}
}

// Build base ALTER TABLE ADD COLUMN statement
// Use newline format if there's an inline constraint for better readability
var stmt string
if constraints.ForeignKey != nil || constraints.PrimaryKey != nil || constraints.Unique != nil {
// Use multi-line format for complex statements with constraints
stmt = fmt.Sprintf("ALTER TABLE %s\nADD COLUMN %s %s",
tableName, ir.QuoteIdentifier(column.Name), columnType)
if inlineConstraint != "" {
stmt = fmt.Sprintf("ALTER TABLE %s\nADD COLUMN %s %s%s%s",
tableName, ir.QuoteIdentifier(column.Name), columnType, clauses, inlineConstraint)
} else {
// Use single-line format for simple column additions
stmt = fmt.Sprintf("ALTER TABLE %s ADD COLUMN %s %s",
tableName, ir.QuoteIdentifier(column.Name), columnType)
stmt = fmt.Sprintf("ALTER TABLE %s ADD COLUMN %s %s%s",
tableName, ir.QuoteIdentifier(column.Name), columnType, clauses)
}

// Build and append all column clauses
clauses := buildColumnClauses(column, constraints, isPartOfAnyPK, td.Table.Schema, targetSchema)
stmt += clauses

context := &diffContext{
Type: DiffTypeTableColumn,
Operation: DiffOperationCreate,
Expand Down Expand Up @@ -601,18 +628,11 @@ func (td *tableDiff) generateAlterTableStatements(targetSchema string, collector

// Add new constraints - already sorted by the Diff operation
for _, constraint := range td.AddedConstraints {
// Skip FK constraints that were already handled inline with column additions
if constraint.Type == ir.ConstraintTypeForeignKey && handledFKConstraints[constraint.Name] {
continue
}
// Skip PK constraints that were already handled inline with column additions
if constraint.Type == ir.ConstraintTypePrimaryKey && handledPKConstraints[constraint.Name] {
continue
}
// Skip UK constraints that were already handled inline with column additions
if constraint.Type == ir.ConstraintTypeUnique && handledUKConstraints[constraint.Name] {
// Skip constraints that were already added inline with columns
if inlineConstraints[constraint.Name] {
continue
}

switch constraint.Type {
case ir.ConstraintTypeUnique:
// Sort columns by position
Expand Down Expand Up @@ -1032,75 +1052,32 @@ func writeColumnDefinitionToBuilder(builder *strings.Builder, table *ir.Table, c

builder.WriteString(dataType)

// Convert table.Constraints map to slice for helper
var allConstraints []*ir.Constraint
for _, constraint := range table.Constraints {
allConstraints = append(allConstraints, constraint)
}

// Find all single-column constraints for this column
// For CREATE TABLE, both FK/CHECK and PK/UK come from table.Constraints
constraints, isPartOfAnyPK := findSingleColumnConstraints(column, allConstraints, allConstraints)

// Build and append all column clauses
clauses := buildColumnClauses(column, constraints, isPartOfAnyPK, table.Schema, targetSchema)
builder.WriteString(clauses)
}

// columnConstraints holds single-column constraints found for a specific column
type columnConstraints struct {
PrimaryKey *ir.Constraint
Unique *ir.Constraint
ForeignKey *ir.Constraint
Checks []*ir.Constraint
}

// findSingleColumnConstraints finds all single-column constraints for a given column
// Also checks if column is part of any (single or multi-column) primary key for NOT NULL handling
func findSingleColumnConstraints(column *ir.Column, allConstraints []*ir.Constraint, pkUKSource []*ir.Constraint) (columnConstraints, bool) {
result := columnConstraints{
Checks: []*ir.Constraint{},
}
// Check if column is part of any primary key constraint for NOT NULL handling
isPartOfAnyPK := false

// Search for FK and CHECK in allConstraints (always from table.Constraints)
for _, constraint := range allConstraints {
if len(constraint.Columns) == 1 && constraint.Columns[0].Name == column.Name {
switch constraint.Type {
case ir.ConstraintTypeForeignKey:
result.ForeignKey = constraint
// CHECK constraints are always handled as table-level constraints
}
}
}

// Search for PK and UK in pkUKSource (could be table.Constraints or td.AddedConstraints)
// Also check if column is part of any primary key (including multi-column PKs)
for _, constraint := range pkUKSource {
for _, constraint := range table.Constraints {
if constraint.Type == ir.ConstraintTypePrimaryKey {
// Check if this column is in this PK constraint
for _, col := range constraint.Columns {
if col.Name == column.Name {
isPartOfAnyPK = true
// Only set PrimaryKey if it's a single-column PK (for inline PRIMARY KEY)
if len(constraint.Columns) == 1 {
result.PrimaryKey = constraint
}
break
}
}
} else if constraint.Type == ir.ConstraintTypeUnique && len(constraint.Columns) == 1 && constraint.Columns[0].Name == column.Name {
result.Unique = constraint
if isPartOfAnyPK {
break
}
}
}

return result, isPartOfAnyPK
// Build and append all column clauses
clauses := buildColumnClauses(column, isPartOfAnyPK, table.Schema, targetSchema)
builder.WriteString(clauses)
}


// buildColumnClauses builds the SQL clauses for a column definition (works for both CREATE TABLE and ALTER TABLE)
// Returns the clauses as a string to be appended to the column name and type
// Order follows PostgreSQL documentation: https://www.postgresql.org/docs/current/sql-altertable.html
func buildColumnClauses(column *ir.Column, constraints columnConstraints, isPartOfAnyPK bool, tableSchema string, targetSchema string) string {
func buildColumnClauses(column *ir.Column, isPartOfAnyPK bool, tableSchema string, targetSchema string) string {
var parts []string

// 1. Identity columns (must come early, before DEFAULT)
Expand Down Expand Up @@ -1144,32 +1121,15 @@ func buildColumnClauses(column *ir.Column, constraints columnConstraints, isPart
parts = append(parts, "NOT NULL")
}

// 5. PRIMARY KEY (must come after GENERATED for generated columns)
if constraints.PrimaryKey != nil {
parts = append(parts, "PRIMARY KEY")
}

// 6. UNIQUE (skip if PRIMARY KEY present, since PK implies UNIQUE)
if constraints.Unique != nil && constraints.PrimaryKey == nil {
parts = append(parts, "UNIQUE")
}
// Note: No inline constraints (PRIMARY KEY, UNIQUE, FOREIGN KEY) are added here
// ALL constraints are now handled as table-level constraints for consistency
// This ensures all constraint names are preserved and provides cleaner formatting

// 7. FOREIGN KEY (REFERENCES)
// Note: generateForeignKeyClause with inline=true already includes the leading space
fkClause := ""
if constraints.ForeignKey != nil {
fkClause = generateForeignKeyClause(constraints.ForeignKey, targetSchema, true)
}

if len(parts) == 0 && fkClause == "" {
if len(parts) == 0 {
return ""
}

result := ""
if len(parts) > 0 {
result = " " + strings.Join(parts, " ")
}
result += fkClause // FK clause already has leading space
result := " " + strings.Join(parts, " ")
return result
}

Expand Down
3 changes: 2 additions & 1 deletion testdata/diff/create_table/add_column_generated/diff.sql
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
ALTER TABLE merge_request
ADD COLUMN iid integer GENERATED ALWAYS AS (CAST(data ->> 'iid' AS int)) STORED PRIMARY KEY;
ADD COLUMN iid integer GENERATED ALWAYS AS (CAST(data ->> 'iid' AS int)) STORED CONSTRAINT pk_merge_request_iid PRIMARY KEY;

ALTER TABLE merge_request ADD COLUMN title text GENERATED ALWAYS AS (data ->> 'title') STORED;
Loading