diff --git a/pkg/diff/diff.go b/pkg/diff/diff.go index 048e71f..b6389d9 100644 --- a/pkg/diff/diff.go +++ b/pkg/diff/diff.go @@ -504,6 +504,9 @@ func compareConstraintDetails(source, target *models.Constraint) map[string]any if normalizeConstraintAction(source.OnUpdate) != normalizeConstraintAction(target.OnUpdate) { changes["on_update"] = map[string]string{"source": source.OnUpdate, "target": target.OnUpdate} } + if !models.DeferralMatches(source, target) { + changes["deferral"] = map[string]string{"source": source.DeferralClause(), "target": target.DeferralClause()} + } return changes } diff --git a/pkg/merge/merge.go b/pkg/merge/merge.go index e32f1df..e0dfbf9 100644 --- a/pkg/merge/merge.go +++ b/pkg/merge/merge.go @@ -605,8 +605,8 @@ func cloneConstraint(constraint *models.Constraint) *models.Constraint { OnDelete: constraint.OnDelete, Expression: constraint.Expression, Name: constraint.Name, - Deferrable: constraint.Deferrable, - InitiallyDeferred: constraint.InitiallyDeferred, + Deferrable: copyBoolPtr(constraint.Deferrable), + InitiallyDeferred: copyBoolPtr(constraint.InitiallyDeferred), Sequence: constraint.Sequence, } copy(newConstraint.Columns, constraint.Columns) @@ -823,3 +823,11 @@ func min(a, b int) int { } return b } + +func copyBoolPtr(b *bool) *bool { + if b == nil { + return nil + } + v := *b + return &v +} diff --git a/pkg/models/deferral.go b/pkg/models/deferral.go new file mode 100644 index 0000000..db7c659 --- /dev/null +++ b/pkg/models/deferral.go @@ -0,0 +1,48 @@ +package models + +// DeferralClause returns the SQL deferral clause for a foreign key constraint +// ("DEFERRABLE", "DEFERRABLE INITIALLY DEFERRED" or "NOT DEFERRABLE"), or "" +// when the model does not specify one. InitiallyDeferred without Deferrable +// implies DEFERRABLE; InitiallyDeferred=false alone specifies nothing. +func (d *Constraint) DeferralClause() string { + deferrable, initially, set := d.effectiveDeferral() + switch { + case !set: + return "" + case !deferrable: + return "NOT DEFERRABLE" + case initially != nil && *initially: + return "DEFERRABLE INITIALLY DEFERRED" + default: + return "DEFERRABLE" + } +} + +// effectiveDeferral resolves the optional flags. set is false when the model says nothing +// about deferral; initially is nil when the initial mode is unspecified. +func (d *Constraint) effectiveDeferral() (deferrable bool, initially *bool, set bool) { + switch { + case d.Deferrable != nil: + return *d.Deferrable, d.InitiallyDeferred, true + case d.InitiallyDeferred != nil && *d.InitiallyDeferred: + return true, d.InitiallyDeferred, true + } + return false, nil, false +} + +// DeferralMatches reports whether two constraints agree on deferral. An unspecified side +// matches anything, and the initial mode is only compared when both sides specify it. +func DeferralMatches(a, b *Constraint) bool { + da, ia, aSet := a.effectiveDeferral() + db, ib, bSet := b.effectiveDeferral() + if !aSet || !bSet { + return true + } + if da != db { + return false + } + if !da || ia == nil || ib == nil { + return true + } + return *ia == *ib +} diff --git a/pkg/models/deferral_test.go b/pkg/models/deferral_test.go new file mode 100644 index 0000000..17f1bc6 --- /dev/null +++ b/pkg/models/deferral_test.go @@ -0,0 +1,59 @@ +package models + +import "testing" + +func boolPtr(b bool) *bool { return &b } + +func TestConstraint_DeferralClause(t *testing.T) { + tests := []struct { + name string + c Constraint + want string + }{ + {"unspecified", Constraint{}, ""}, + {"deferrable", Constraint{Deferrable: boolPtr(true)}, "DEFERRABLE"}, + {"deferrable initially deferred", Constraint{Deferrable: boolPtr(true), InitiallyDeferred: boolPtr(true)}, "DEFERRABLE INITIALLY DEFERRED"}, + {"deferrable initially immediate", Constraint{Deferrable: boolPtr(true), InitiallyDeferred: boolPtr(false)}, "DEFERRABLE"}, + {"initially deferred implies deferrable", Constraint{InitiallyDeferred: boolPtr(true)}, "DEFERRABLE INITIALLY DEFERRED"}, + {"initially immediate alone is unspecified", Constraint{InitiallyDeferred: boolPtr(false)}, ""}, + {"explicitly not deferrable", Constraint{Deferrable: boolPtr(false)}, "NOT DEFERRABLE"}, + {"not deferrable ignores initially deferred", Constraint{Deferrable: boolPtr(false), InitiallyDeferred: boolPtr(true)}, "NOT DEFERRABLE"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.c.DeferralClause(); got != tt.want { + t.Errorf("DeferralClause() = %q, want %q", got, tt.want) + } + }) + } +} + +func TestDeferralMatches(t *testing.T) { + live := func(def, init bool) *Constraint { + return &Constraint{Deferrable: boolPtr(def), InitiallyDeferred: boolPtr(init)} + } + tests := []struct { + name string + model, other *Constraint + want bool + }{ + {"model unspecified vs deferrable live", &Constraint{}, live(true, false), true}, + {"model unspecified vs not deferrable live", &Constraint{}, live(false, false), true}, + {"deferrable vs deferrable live", &Constraint{Deferrable: boolPtr(true)}, live(true, true), true}, + {"deferrable vs not deferrable live", &Constraint{Deferrable: boolPtr(true)}, live(false, false), false}, + {"not deferrable vs deferrable live", &Constraint{Deferrable: boolPtr(false)}, live(true, false), false}, + {"initially deferred vs initially immediate live", &Constraint{InitiallyDeferred: boolPtr(true)}, live(true, false), false}, + {"initially deferred vs initially deferred live", &Constraint{InitiallyDeferred: boolPtr(true)}, live(true, true), true}, + {"explicit initially immediate vs initially deferred live", &Constraint{Deferrable: boolPtr(true), InitiallyDeferred: boolPtr(false)}, live(true, true), false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := DeferralMatches(tt.model, tt.other); got != tt.want { + t.Errorf("DeferralMatches() = %v, want %v", got, tt.want) + } + if got := DeferralMatches(tt.other, tt.model); got != tt.want { + t.Errorf("DeferralMatches() reversed = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pkg/models/models.go b/pkg/models/models.go index 8027f6f..8b183e1 100644 --- a/pkg/models/models.go +++ b/pkg/models/models.go @@ -329,8 +329,8 @@ type Constraint struct { ReferencedColumns []string `json:"referenced_columns" yaml:"referenced_columns" xml:"referenced_columns"` OnDelete string `json:"on_delete" yaml:"on_delete" xml:"on_delete"` // CASCADE, SET NULL, RESTRICT, etc. OnUpdate string `json:"on_update" yaml:"on_update" xml:"on_update"` - Deferrable bool `json:"deferrable,omitempty" yaml:"deferrable,omitempty" xml:"deferrable,omitempty"` - InitiallyDeferred bool `json:"initially_deferred,omitempty" yaml:"initially_deferred,omitempty" xml:"initially_deferred,omitempty"` + Deferrable *bool `json:"deferrable,omitempty" yaml:"deferrable,omitempty" xml:"deferrable,omitempty"` // nil: unspecified, left as is + InitiallyDeferred *bool `json:"initially_deferred,omitempty" yaml:"initially_deferred,omitempty" xml:"initially_deferred,omitempty"` // nil: unspecified, left as is Sequence uint `json:"sequence,omitempty" yaml:"sequence,omitempty" xml:"sequence,omitempty"` GUID string `json:"guid" yaml:"guid" xml:"guid"` } diff --git a/pkg/readers/dbml/README.md b/pkg/readers/dbml/README.md index d4fc313..ea4ea9b 100644 --- a/pkg/readers/dbml/README.md +++ b/pkg/readers/dbml/README.md @@ -86,16 +86,40 @@ Ref: posts.user_id > users.id [delete: cascade] - Table definitions with columns - Primary keys (`pk`) - Not null constraints (`not null`) -- Unique constraints (`unique`) +- Unique constraints (`unique`) → constraint `ukey__` - Default values (`default`) - Inline references (`ref`) - Standalone `Ref` blocks +- FK deferral: `deferrable`, `initially deferred` (see below) - Commented cross-file refs (`// Ref:` — see below) - Indexes and composite indexes - Table notes and column notes - Enums - Dialect directives (`@postgres:` / `@sqlite:` — see below) +## Naming + +| Source | Generated name | +| ------ | -------------- | +| Column `[unique]` | constraint `ukey_
_` | +| Unnamed unique index | index `ukey_
_` (same name as the constraint on the same columns) | +| Unnamed index | `idx_
_` | + +Explicit `name:` is never changed. Leading `_` in the table name is stripped. + +## FK deferral + +``` +col int [ref: > t.id, deferrable, initially deferred] +Ref: a.x > b.id [delete: cascade, deferrable] +``` + +| Setting | SQL | +| ------- | --- | +| `deferrable` | `DEFERRABLE` | +| `initially deferred` | `DEFERRABLE INITIALLY DEFERRED` | +| neither | unspecified (existing FK left as is) | + ## Commented cross-file refs `// Ref:` / `// ref:` lines (ignored by dbdiagram) become FKs + relationships once both ends are loaded. diff --git a/pkg/readers/dbml/reader.go b/pkg/readers/dbml/reader.go index 317c117..9b109b0 100644 --- a/pkg/readers/dbml/reader.go +++ b/pkg/readers/dbml/reader.go @@ -848,6 +848,13 @@ func (r *Reader) parseColumn(line, tableName, schemaName string) (*models.Column } } + // "deferrable" / "initially deferred" column settings apply to the inline foreign key + if constraint != nil && constraint.Type == models.ForeignKeyConstraint && attrs != "" { + for _, attr := range splitColumnAttrs(attrs) { + applyDeferralSetting(constraint, attr) + } + } + // Parse inline comment if inlineComment != "" { column.Comment = inlineComment @@ -1081,11 +1088,13 @@ func (r *Reader) parseIndex(line, tableName, schemaName string) *models.Index { // Generate name if not provided if index.Name == "" { - prefix := "idx" + // Unique indexes share the ukey_ naming of unique constraints, so an index and a + // constraint on the same columns get the same name and can be merged on write. + prefix, name := "idx", tableName if index.Unique { - prefix = "uidx" + prefix, name = "ukey", strings.TrimLeft(tableName, "_") } - index.Name = fmt.Sprintf("%s_%s_%s", prefix, tableName, strings.Join(columns, "_")) + index.Name = fmt.Sprintf("%s_%s_%s", prefix, name, strings.Join(columns, "_")) } return index @@ -1182,6 +1191,8 @@ func (r *Reader) parseRef(refStr string) *models.Constraint { constraint.OnDelete = strings.TrimSpace(strings.TrimPrefix(action, "ondelete:")) } else if strings.HasPrefix(action, "onupdate:") { constraint.OnUpdate = strings.TrimSpace(strings.TrimPrefix(action, "onupdate:")) + } else { + applyDeferralSetting(constraint, action) } } } @@ -1190,6 +1201,18 @@ func (r *Reader) parseRef(refStr string) *models.Constraint { return constraint } +// applyDeferralSetting applies a "deferrable" or "initially deferred" Ref setting to a +// foreign key constraint. Other settings are ignored. +func applyDeferralSetting(constraint *models.Constraint, setting string) { + yes := true + switch strings.Join(strings.Fields(strings.ToLower(setting)), " ") { + case "deferrable": + constraint.Deferrable = &yes + case "initially deferred", "initially_deferred": + constraint.InitiallyDeferred = &yes + } +} + // parseTableRef parses a table reference like "schema.table.(column1, column2)" or "schema"."table"."column" func (r *Reader) parseTableRef(ref string) (schema, table string, columns []string) { // Extract columns if present in parentheses format diff --git a/pkg/readers/dbml/reader_test.go b/pkg/readers/dbml/reader_test.go index eb60b7a..a6069a1 100644 --- a/pkg/readers/dbml/reader_test.go +++ b/pkg/readers/dbml/reader_test.go @@ -3,6 +3,7 @@ package dbml import ( "os" "path/filepath" + "strings" "testing" "git.warky.dev/wdevs/relspecgo/pkg/models" @@ -842,9 +843,9 @@ func TestConstraintNaming(t *testing.T) { t.Logf("Available constraints: %v", getKeys(postsTable.Constraints)) } - // Test unique index naming: uidx_table_columns - if _, exists := postsTable.Indexes["uidx_posts_slug"]; !exists { - t.Error("Expected unique index 'uidx_posts_slug' not found") + // Test unique index naming: ukey_table_columns, matching the unique constraint name + if _, exists := postsTable.Indexes["ukey_posts_slug"]; !exists { + t.Error("Expected unique index 'ukey_posts_slug' not found") t.Logf("Available indexes: %v", getKeys(postsTable.Indexes)) } @@ -1121,3 +1122,69 @@ func TestReader_DuplicateIndexNameInTableKept(t *testing.T) { } } } + +func TestReader_ForeignKeyDeferral(t *testing.T) { + dbml := `Table users { + id integer [pk] +} + +Table posts { + id integer [pk] + user_id integer [ref: > users.id, deferrable, initially deferred] + editor_id integer + plain_id integer [ref: > users.id] +} + +Table comments { + id integer [pk] + post_id integer + author_id integer +} + +Ref: comments.post_id > posts.id [delete: cascade, deferrable] +Ref: comments.author_id > users.id [deferrable, initially deferred] +` + path := filepath.Join(t.TempDir(), "deferral.dbml") + if err := os.WriteFile(path, []byte(dbml), 0o600); err != nil { + t.Fatal(err) + } + db, err := NewReader(&readers.ReaderOptions{FilePath: path}).ReadDatabase() + if err != nil { + t.Fatalf("ReadDatabase() error = %v", err) + } + + find := func(table, column string) *models.Constraint { + for _, s := range db.Schemas { + for _, tb := range s.Tables { + if tb.Name != table { + continue + } + for _, c := range tb.Constraints { + if c.Type == models.ForeignKeyConstraint && len(c.Columns) == 1 && c.Columns[0] == column { + return c + } + } + } + } + t.Fatalf("FK %s.%s not found", table, column) + return nil + } + + tests := []struct { + table, column string + want string + }{ + {"posts", "user_id", "DEFERRABLE INITIALLY DEFERRED"}, + {"posts", "plain_id", ""}, + {"comments", "post_id", "DEFERRABLE"}, + {"comments", "author_id", "DEFERRABLE INITIALLY DEFERRED"}, + } + for _, tt := range tests { + if got := find(tt.table, tt.column).DeferralClause(); got != tt.want { + t.Errorf("%s.%s deferral = %q, want %q", tt.table, tt.column, got, tt.want) + } + } + if got := find("comments", "post_id").OnDelete; !strings.EqualFold(got, "cascade") { + t.Errorf("delete action lost, got %q", got) + } +} diff --git a/pkg/readers/pgsql/queries.go b/pkg/readers/pgsql/queries.go index b11094c..63853a6 100644 --- a/pkg/readers/pgsql/queries.go +++ b/pkg/readers/pgsql/queries.go @@ -439,7 +439,9 @@ func (r *Reader) queryForeignKeys(schemaName string) (map[string][]*models.Const fcl.relname AS referenced_table_name, fatt.attname AS referenced_column, ` + actionCase("con.confupdtype") + ` AS update_rule, - ` + actionCase("con.confdeltype") + ` AS delete_rule + ` + actionCase("con.confdeltype") + ` AS delete_rule, + con.condeferrable AS deferrable, + con.condeferred AS initially_deferred FROM pg_catalog.pg_constraint con JOIN pg_catalog.pg_class cl ON cl.oid = con.conrelid JOIN pg_catalog.pg_namespace ns ON ns.oid = cl.relnamespace @@ -470,6 +472,8 @@ func (r *Reader) queryForeignKeys(schemaName string) (map[string][]*models.Const referencedColumns []string updateRule string deleteRule string + deferrable bool + initiallyDeferred bool } fkMap := make(map[string]*fkData) @@ -479,8 +483,9 @@ func (r *Reader) queryForeignKeys(schemaName string) (map[string][]*models.Const var foreignSchema, foreignTable, foreignColumn string var referencedSchema, referencedTable, referencedColumn string var updateRule, deleteRule string + var deferrable, initiallyDeferred bool - if err := rows.Scan(&schema, &tableName, &constraintName, &foreignSchema, &foreignTable, &foreignColumn, &referencedSchema, &referencedTable, &referencedColumn, &updateRule, &deleteRule); err != nil { + if err := rows.Scan(&schema, &tableName, &constraintName, &foreignSchema, &foreignTable, &foreignColumn, &referencedSchema, &referencedTable, &referencedColumn, &updateRule, &deleteRule, &deferrable, &initiallyDeferred); err != nil { return nil, err } @@ -497,6 +502,8 @@ func (r *Reader) queryForeignKeys(schemaName string) (map[string][]*models.Const referencedColumns: []string{}, updateRule: updateRule, deleteRule: deleteRule, + deferrable: deferrable, + initiallyDeferred: initiallyDeferred, } } @@ -517,6 +524,9 @@ func (r *Reader) queryForeignKeys(schemaName string) (map[string][]*models.Const constraint.ReferencedColumns = fk.referencedColumns constraint.OnUpdate = fk.updateRule constraint.OnDelete = fk.deleteRule + deferrable, initiallyDeferred := fk.deferrable, fk.initiallyDeferred + constraint.Deferrable = &deferrable + constraint.InitiallyDeferred = &initiallyDeferred tableKey := fk.schema + "." + fk.tableName foreignKeys[tableKey] = append(foreignKeys[tableKey], constraint) diff --git a/pkg/writers/dbml/writer.go b/pkg/writers/dbml/writer.go index 4a7c581..e4e25e2 100644 --- a/pkg/writers/dbml/writer.go +++ b/pkg/writers/dbml/writer.go @@ -273,6 +273,13 @@ func (w *Writer) constraintToDBML(c *models.Constraint, t *models.Table) string actions = append(actions, fmt.Sprintf("update: %s", c.OnUpdate)) } + if c.Deferrable != nil && *c.Deferrable { + actions = append(actions, "deferrable") + } + if c.InitiallyDeferred != nil && *c.InitiallyDeferred { + actions = append(actions, "initially deferred") + } + refLine := fmt.Sprintf("Ref: %s %s %s", fromRef, relationship, toRef) if len(actions) > 0 { refLine += fmt.Sprintf(" [%s]", strings.Join(actions, ", ")) diff --git a/pkg/writers/dbml/writer_test.go b/pkg/writers/dbml/writer_test.go index 6303b37..156ddf7 100644 --- a/pkg/writers/dbml/writer_test.go +++ b/pkg/writers/dbml/writer_test.go @@ -171,3 +171,31 @@ func TestWriter_WriteDatabase_OneToOneRelationship(t *testing.T) { assert.Contains(t, output, "Ref: public.profiles.user_id - public.users.id") } + +func TestWriter_WriteDatabase_ForeignKeyDeferral(t *testing.T) { + yes := true + db := models.InitDatabase("test_db") + schema := models.InitSchema("public") + users := models.InitTable("users", "public") + users.Columns["id"] = models.InitColumn("id", "users", "public") + users.Columns["id"].Type = "bigint" + posts := models.InitTable("posts", "public") + posts.Columns["user_id"] = models.InitColumn("user_id", "posts", "public") + posts.Columns["user_id"].Type = "bigint" + fk := models.InitConstraint("fk_posts_user", models.ForeignKeyConstraint) + fk.Table, fk.Schema = "posts", "public" + fk.Columns = []string{"user_id"} + fk.ReferencedTable, fk.ReferencedSchema = "users", "public" + fk.ReferencedColumns = []string{"id"} + fk.Deferrable = &yes + fk.InitiallyDeferred = &yes + posts.Constraints[fk.Name] = fk + schema.Tables = append(schema.Tables, users, posts) + db.Schemas = append(db.Schemas, schema) + + outputPath := filepath.Join(t.TempDir(), "test.dbml") + assert.NoError(t, NewWriter(&writers.WriterOptions{OutputPath: outputPath}).WriteDatabase(db)) + content, err := os.ReadFile(outputPath) + assert.NoError(t, err) + assert.Contains(t, string(content), "Ref: public.posts.user_id > public.users.id [deferrable, initially deferred]") +} diff --git a/pkg/writers/pgsql/NAMING_CONVENTIONS.md b/pkg/writers/pgsql/NAMING_CONVENTIONS.md index db6e04b..876729f 100644 --- a/pkg/writers/pgsql/NAMING_CONVENTIONS.md +++ b/pkg/writers/pgsql/NAMING_CONVENTIONS.md @@ -8,8 +8,8 @@ Standardized naming rules for all database objects in RelSpec PostgreSQL output. | ----------------- | ----------- | ---------------------------------- | ------------------------ | | Primary Key | `pk_` | `pk__
` | `pk_public_users` | | Foreign Key | `fk_` | `fk_
_` | `fk_posts_users` | -| Unique Constraint | `uk_` | `uk_
_` | `uk_users_email` | -| Unique Index | `uidx_` | `uidx_
_` | `uidx_users_email` | +| Unique Constraint | `ukey_` | `ukey_
_` | `ukey_users_email` | +| Unique Index | `ukey_` | `ukey_
_` | `ukey_users_email` | | Regular Index | `idx_` | `idx_
_` | `idx_posts_user_id` | | Check Constraint | `chk_` | `chk_
_` | `chk_users_age_positive` | | Sequence | `identity_` | `identity_
_` | `identity_users_id` | @@ -43,25 +43,25 @@ Standardized naming rules for all database objects in RelSpec PostgreSQL output. ### Unique Constraints -**Pattern:** `uk_
_` +**Pattern:** `ukey_
_` -- Use `uk_` prefix strictly for database constraints (CONSTRAINT type) +- Generated by the DBML reader for column `[unique]` - Include column name for clarity - Examples: - - `uk_users_email` - - `uk_users_username` - - `uk_products_sku` + - `ukey_users_email` + - `ukey_users_username` + - `ukey_products_sku` ### Unique Indexes -**Pattern:** `uidx_
_` +**Pattern:** `ukey_
_` -- Use `uidx_` prefix strictly for index type objects -- Distinguished from constraints for clarity and implementation flexibility +- Unnamed unique indexes use the same name as a unique constraint on the same columns +- Writers then build the constraint on the index (`ADD CONSTRAINT ... UNIQUE USING INDEX`), so no duplicate index is created - Examples: - - `uidx_users_email` - - `uidx_sessions_token` - - `uidx_api_keys_key` + - `ukey_users_email` + - `ukey_sessions_token` + - `ukey_api_keys_key` ### Regular Indexes diff --git a/pkg/writers/pgsql/README.md b/pkg/writers/pgsql/README.md index 564e49f..3c11a7b 100644 --- a/pkg/writers/pgsql/README.md +++ b/pkg/writers/pgsql/README.md @@ -154,6 +154,29 @@ When `migration_mode` is enabled, generates migration scripts with: - Transactional DDL - Rollback support +### Unique constraints and indexes + +- Unique constraint → `ADD CONSTRAINT UNIQUE (cols)`, idempotent (checked by name) +- Migration mode and direct apply both emit them (not only `convert`) +- Same-named model unique index → `UNIQUE USING INDEX` (no duplicate index). Requires: same name, same columns, btree, non-partial, no `INCLUDE` +- Otherwise a plain `ADD CONSTRAINT` (second index is built) +- Naming: unique constraint and unique index both `ukey_
_`; see `NAMING_CONVENTIONS.md` +- Renaming `uidx_…` → `ukey_…` drops the old index and re-adds the constraint on the next diff + +### FK deferral + +| Model (`Deferrable` / `InitiallyDeferred`) | SQL | +| ------------------------------------------ | --- | +| unset | create: `DEFERRABLE` (migration/apply), none (full DDL constraints); existing FK never diffed | +| `true` / unset or `false` | `DEFERRABLE` | +| `true` / `true`, or unset / `true` | `DEFERRABLE INITIALLY DEFERRED` | +| `false` | `NOT DEFERRABLE` | + +- Both fields are `*bool`; `nil` = unspecified +- Specified value differing from the live DB → FK dropped and re-added +- Live reader reads `condeferrable` / `condeferred` +- Initial mode compared only when both sides state it + ### Audit Triggers When `include_audit` is enabled, adds: diff --git a/pkg/writers/pgsql/migration_writer.go b/pkg/writers/pgsql/migration_writer.go index 1d58824..0fb1b77 100644 --- a/pkg/writers/pgsql/migration_writer.go +++ b/pkg/writers/pgsql/migration_writer.go @@ -222,6 +222,13 @@ func (w *MigrationWriter) generateSchemaScripts(model, current *models.Schema) ( } scripts = append(scripts, indexScripts...) + // Phase 4.5: Create unique constraints (Priority 185) + uniqueScripts, err := w.generateUniqueConstraintScripts(model, current) + if err != nil { + return nil, fmt.Errorf("failed to generate unique constraint scripts: %w", err) + } + scripts = append(scripts, uniqueScripts...) + // Phase 5: Create foreign keys (Priority 195) fkScripts, err := w.generateForeignKeyScripts(model, current, droppedFKs) if err != nil { @@ -751,6 +758,92 @@ func buildIndexColumnExpressionsFiltered(table *models.Table, index *models.Inde return columnExprs } +// generateUniqueConstraintScripts generates ADD CONSTRAINT UNIQUE scripts for model unique +// constraints that are missing from, or differ in, the live schema. +func (w *MigrationWriter) generateUniqueConstraintScripts(model, current *models.Schema) ([]MigrationScript, error) { + scripts := make([]MigrationScript, 0) + + currentTables := make(map[string]*models.Table) + if current != nil { + for _, table := range current.Tables { + currentTables[strings.ToLower(table.Name)] = table + } + } + + for _, modelTable := range model.Tables { + currentTable := currentTables[strings.ToLower(modelTable.Name)] + + for _, constraint := range sortConstraints(modelTable.Constraints) { + if constraint.Type != models.UniqueConstraint { + continue + } + + if currentTable != nil { + if currentConstraint, exists := lookupConstraint(currentTable.Constraints, constraint.Name); exists && constraintsEqual(constraint, currentConstraint) { + continue + } + } + + columnExprs := make([]string, 0, len(constraint.Columns)) + for _, colName := range constraint.Columns { + if col, ok := modelTable.Columns[colName]; ok { + columnExprs = append(columnExprs, col.SQLName()) + } + } + if len(columnExprs) == 0 { + continue + } + + sql, err := w.executor.ExecuteCreateUniqueConstraint(CreateUniqueConstraintData{ + SchemaName: model.Name, + TableName: modelTable.Name, + ConstraintName: constraint.Name, + Columns: strings.Join(columnExprs, ", "), + UsingIndex: adoptableUniqueIndex(modelTable, constraint), + }) + if err != nil { + return nil, err + } + + scripts = append(scripts, MigrationScript{ + ObjectName: fmt.Sprintf("%s.%s.%s", model.Name, modelTable.Name, constraint.Name), + ObjectType: "create unique constraint", + Schema: model.Name, + Priority: 185, + Sequence: len(scripts), + Body: sql, + }) + } + } + + return scripts, nil +} + +// adoptableUniqueIndex returns the name of the model's unique index that the given unique +// constraint can be built on with ADD CONSTRAINT ... UNIQUE USING INDEX, so the constraint does +// not create a second index (or collide with the index's name). The index must carry the +// constraint's name, be a plain non-partial btree index and cover exactly the constraint's +// columns. Index creation runs before constraint creation, so this holds whether or not the +// live database already has the index. Returns "" when none qualifies. +func adoptableUniqueIndex(table *models.Table, constraint *models.Constraint) string { + index, ok := table.Indexes[constraint.Name] + if !ok || !index.Unique || index.Where != "" || len(index.Include) > 0 { + return "" + } + if index.Type != "" && !strings.EqualFold(index.Type, "btree") { + return "" + } + if len(index.Columns) != len(constraint.Columns) { + return "" + } + for i, col := range index.Columns { + if !strings.EqualFold(col, constraint.Columns[i]) { + return "" + } + } + return index.Name +} + // generateForeignKeyScripts generates ADD CONSTRAINT FOREIGN KEY scripts using templates. // forceRecreate is a set of FK constraint keys (schema.table.name) that must be recreated // even if unchanged, because their referenced PK was dropped and recreated. @@ -800,6 +893,12 @@ func (w *MigrationWriter) generateForeignKeyScripts(model, current *models.Schem onUpdate = strings.ToUpper(constraint.OnUpdate) } + // An unspecified deferral keeps the historical default of DEFERRABLE. + deferral := constraint.DeferralClause() + if deferral == "" { + deferral = "DEFERRABLE" + } + sql, err := w.executor.ExecuteCreateForeignKey(CreateForeignKeyData{ SchemaName: model.Name, TableName: modelTable.Name, @@ -810,6 +909,7 @@ func (w *MigrationWriter) generateForeignKeyScripts(model, current *models.Schem TargetColumns: strings.Join(constraint.ReferencedColumns, ", "), OnDelete: onDelete, OnUpdate: onUpdate, + Deferral: deferral, }) if err != nil { return nil, err @@ -1149,6 +1249,9 @@ func constraintsEqual(c1, c2 *models.Constraint) bool { if !fkActionsEqual(c1.OnDelete, c2.OnDelete) || !fkActionsEqual(c1.OnUpdate, c2.OnUpdate) { return false } + if !models.DeferralMatches(c1, c2) { + return false + } } return true diff --git a/pkg/writers/pgsql/migration_writer_test.go b/pkg/writers/pgsql/migration_writer_test.go index 30386a3..812944f 100644 --- a/pkg/writers/pgsql/migration_writer_test.go +++ b/pkg/writers/pgsql/migration_writer_test.go @@ -1027,3 +1027,316 @@ func TestIndexesEqual_OperatorClassAndStorageParameters(t *testing.T) { }) } } + +func uniqueConstraintTestModel() *models.Database { + model := models.InitDatabase("testdb") + schema := models.InitSchema("public") + + table := models.InitTable("login", "public") + idCol := models.InitColumn("id", "login", "public") + idCol.Type = "integer" + idCol.NotNull = true + table.Columns["id"] = idCol + guidCol := models.InitColumn("guid", "login", "public") + guidCol.Type = "uuid" + table.Columns["guid"] = guidCol + + uc := models.InitConstraint("ukey_login_guid", models.UniqueConstraint) + uc.Schema = "public" + uc.Table = "login" + uc.Columns = []string{"guid"} + table.Constraints[uc.Name] = uc + + schema.Tables = append(schema.Tables, table) + model.Schemas = append(model.Schemas, schema) + return model +} + +func TestWriteMigration_UniqueConstraint(t *testing.T) { + tests := []struct { + name string + current func() *models.Database + wantConstrnt bool + }{ + { + name: "new table", + current: func() *models.Database { + c := models.InitDatabase("testdb") + c.Schemas = append(c.Schemas, models.InitSchema("public")) + return c + }, + wantConstrnt: true, + }, + { + name: "existing table without constraint", + current: func() *models.Database { + c := uniqueConstraintTestModel() + delete(c.Schemas[0].Tables[0].Constraints, "ukey_login_guid") + return c + }, + wantConstrnt: true, + }, + { + name: "existing table with constraint", + current: uniqueConstraintTestModel, + wantConstrnt: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var buf bytes.Buffer + writer, err := NewMigrationWriter(&writers.WriterOptions{}) + if err != nil { + t.Fatalf("Failed to create writer: %v", err) + } + writer.writer = &buf + + if err := writer.WriteMigration(uniqueConstraintTestModel(), tt.current()); err != nil { + t.Fatalf("WriteMigration failed: %v", err) + } + + output := buf.String() + has := strings.Contains(output, "ADD CONSTRAINT") && strings.Contains(output, "ukey_login_guid") && strings.Contains(output, "UNIQUE") + if has != tt.wantConstrnt { + t.Fatalf("unique constraint present = %v, want %v; output:\n%s", has, tt.wantConstrnt, output) + } + if strings.Contains(output, "DROP CONSTRAINT") { + t.Fatalf("unexpected DROP CONSTRAINT; output:\n%s", output) + } + }) + } +} + +func TestWriteMigration_UniqueConstraintAdoptsSameNamedModelIndex(t *testing.T) { + newIndex := func(name string, unique bool, where string) *models.Index { + idx := models.InitIndex(name, "login", "public") + idx.Columns = []string{"guid"} + idx.Unique = unique + idx.Type = "btree" + idx.Where = where + return idx + } + + tests := []struct { + name string + modelIdx *models.Index + liveIdx *models.Index // nil: fresh table + wantAdopt bool + wantDrop bool + }{ + { + name: "fresh table: same-named unique index is promoted", + modelIdx: newIndex("ukey_login_guid", true, ""), + wantAdopt: true, + }, + { + name: "existing live unique index is promoted", + modelIdx: newIndex("ukey_login_guid", true, ""), + liveIdx: newIndex("ukey_login_guid", true, ""), + wantAdopt: true, + }, + { + name: "live unique index not in model is dropped and constraint built fresh", + liveIdx: newIndex("ukey_login_guid", true, ""), + wantDrop: true, + }, + { + name: "partial model index is not promoted", + modelIdx: newIndex("ukey_login_guid", true, "guid IS NOT NULL"), + }, + { + name: "non-unique same-named index is not promoted", + modelIdx: newIndex("ukey_login_guid", false, ""), + }, + { + name: "differently named index is not promoted", + modelIdx: newIndex("idx_login_guid", true, ""), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + model := uniqueConstraintTestModel() + if tt.modelIdx != nil { + model.Schemas[0].Tables[0].Indexes[tt.modelIdx.Name] = tt.modelIdx + } + + current := models.InitDatabase("testdb") + current.Schemas = append(current.Schemas, models.InitSchema("public")) + if tt.liveIdx != nil { + current = uniqueConstraintTestModel() + curTable := current.Schemas[0].Tables[0] + delete(curTable.Constraints, "ukey_login_guid") + curTable.Indexes[tt.liveIdx.Name] = tt.liveIdx + } + + var buf bytes.Buffer + writer, err := NewMigrationWriter(&writers.WriterOptions{}) + if err != nil { + t.Fatalf("Failed to create writer: %v", err) + } + writer.writer = &buf + if err := writer.WriteMigration(model, current); err != nil { + t.Fatalf("WriteMigration failed: %v", err) + } + output := buf.String() + + if got := strings.Contains(output, "UNIQUE USING INDEX"); got != tt.wantAdopt { + t.Errorf("USING INDEX present = %v, want %v; output:\n%s", got, tt.wantAdopt, output) + } + if got := strings.Contains(output, "UNIQUE (guid)") || strings.Contains(output, "UNIQUE (\"guid\")"); got == tt.wantAdopt { + t.Errorf("column-list ADD CONSTRAINT present = %v, want %v; output:\n%s", got, !tt.wantAdopt, output) + } + if got := strings.Contains(output, "DROP INDEX"); got != tt.wantDrop { + t.Errorf("DROP INDEX present = %v, want %v; output:\n%s", got, tt.wantDrop, output) + } + }) + } +} + +func TestWriteDatabase_UniqueConstraintAdoptsSameNamedIndex(t *testing.T) { + db := uniqueConstraintTestModel() + idx := models.InitIndex("ukey_login_guid", "login", "public") + idx.Columns = []string{"guid"} + idx.Unique = true + db.Schemas[0].Tables[0].Indexes[idx.Name] = idx + + var buf bytes.Buffer + w := NewWriter(&writers.WriterOptions{}) + w.writer = &buf + if err := w.WriteDatabase(db); err != nil { + t.Fatalf("WriteDatabase failed: %v", err) + } + output := buf.String() + + if !strings.Contains(output, "UNIQUE USING INDEX") { + t.Fatalf("expected UNIQUE USING INDEX in output:\n%s", output) + } + if strings.Index(output, "CREATE UNIQUE INDEX") > strings.Index(output, "UNIQUE USING INDEX") { + t.Fatalf("index must be created before the constraint adopts it:\n%s", output) + } +} + +func fkDeferralTestModel(def, init *bool) *models.Database { + db := models.InitDatabase("testdb") + schema := models.InitSchema("public") + + users := models.InitTable("users", "public") + uid := models.InitColumn("id", "users", "public") + uid.Type = "integer" + uid.NotNull = true + users.Columns["id"] = uid + + posts := models.InitTable("posts", "public") + pid := models.InitColumn("id", "posts", "public") + pid.Type = "integer" + pid.NotNull = true + posts.Columns["id"] = pid + puid := models.InitColumn("user_id", "posts", "public") + puid.Type = "integer" + posts.Columns["user_id"] = puid + + fk := models.InitConstraint("fk_posts_user_id", models.ForeignKeyConstraint) + fk.Schema = "public" + fk.Table = "posts" + fk.Columns = []string{"user_id"} + fk.ReferencedSchema = "public" + fk.ReferencedTable = "users" + fk.ReferencedColumns = []string{"id"} + fk.Deferrable = def + fk.InitiallyDeferred = init + posts.Constraints[fk.Name] = fk + + schema.Tables = append(schema.Tables, users, posts) + db.Schemas = append(db.Schemas, schema) + return db +} + +func migrationOutput(t *testing.T, model, current *models.Database) string { + t.Helper() + var buf bytes.Buffer + writer, err := NewMigrationWriter(&writers.WriterOptions{}) + if err != nil { + t.Fatalf("Failed to create writer: %v", err) + } + writer.writer = &buf + if err := writer.WriteMigration(model, current); err != nil { + t.Fatalf("WriteMigration failed: %v", err) + } + return buf.String() +} + +func TestWriteMigration_ForeignKeyDeferral(t *testing.T) { + yes, no := true, false + empty := func() *models.Database { + c := models.InitDatabase("testdb") + c.Schemas = append(c.Schemas, models.InitSchema("public")) + return c + } + + tests := []struct { + name string + def *bool + init *bool + wantFrag string + }{ + {"unspecified keeps DEFERRABLE default", nil, nil, "ON UPDATE NO ACTION\n DEFERRABLE;"}, + {"deferrable", &yes, nil, "DEFERRABLE;"}, + {"initially deferred", &yes, &yes, "DEFERRABLE INITIALLY DEFERRED;"}, + {"not deferrable", &no, nil, "NOT DEFERRABLE;"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + out := migrationOutput(t, fkDeferralTestModel(tt.def, tt.init), empty()) + if !strings.Contains(out, tt.wantFrag) { + t.Fatalf("expected %q in output:\n%s", tt.wantFrag, out) + } + }) + } +} + +func TestWriteMigration_ForeignKeyDeferralDiff(t *testing.T) { + yes, no := true, false + live := func(def, init bool) *models.Database { + db := fkDeferralTestModel(&def, &init) + return db + } + + tests := []struct { + name string + def, init *bool + liveDef bool + liveInit bool + wantRecreate bool + }{ + {"unspecified vs deferrable live: untouched", nil, nil, true, false, false}, + {"unspecified vs not deferrable live: untouched", nil, nil, false, false, false}, + {"deferrable matches live", &yes, nil, true, true, false}, + {"deferrable vs not deferrable live: recreated", &yes, nil, false, false, true}, + {"not deferrable vs deferrable live: recreated", &no, nil, true, false, true}, + {"initially deferred vs initially immediate live: recreated", &yes, &yes, true, false, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + out := migrationOutput(t, fkDeferralTestModel(tt.def, tt.init), live(tt.liveDef, tt.liveInit)) + got := strings.Contains(out, "ADD CONSTRAINT") && strings.Contains(out, "FOREIGN KEY") + if got != tt.wantRecreate { + t.Fatalf("FK recreated = %v, want %v; output:\n%s", got, tt.wantRecreate, out) + } + }) + } +} + +func TestWriteDatabase_ForeignKeyDeferral(t *testing.T) { + yes := true + var buf bytes.Buffer + w := NewWriter(&writers.WriterOptions{}) + w.writer = &buf + if err := w.WriteDatabase(fkDeferralTestModel(&yes, &yes)); err != nil { + t.Fatalf("WriteDatabase failed: %v", err) + } + if !strings.Contains(buf.String(), "DEFERRABLE INITIALLY DEFERRED;") { + t.Fatalf("expected DEFERRABLE INITIALLY DEFERRED in output:\n%s", buf.String()) + } +} diff --git a/pkg/writers/pgsql/templates.go b/pkg/writers/pgsql/templates.go index 26b26f5..c730422 100644 --- a/pkg/writers/pgsql/templates.go +++ b/pkg/writers/pgsql/templates.go @@ -164,6 +164,7 @@ type CreateForeignKeyData struct { TargetColumns string OnDelete string OnUpdate string + Deferral string // DEFERRABLE / DEFERRABLE INITIALLY DEFERRED / NOT DEFERRABLE, or empty } // DropConstraintData contains data for drop constraint template @@ -238,6 +239,7 @@ type CreateUniqueConstraintData struct { TableName string ConstraintName string Columns string + UsingIndex string // when set, promotes this existing unique index instead of building a new one } // CreateCheckConstraintData contains data for create check constraint template @@ -259,7 +261,7 @@ type CreateForeignKeyWithCheckData struct { TargetColumns string OnDelete string OnUpdate string - Deferrable bool + Deferral string // DEFERRABLE / DEFERRABLE INITIALLY DEFERRED / NOT DEFERRABLE, or empty } // SetSequenceValueData contains data for set sequence value template diff --git a/pkg/writers/pgsql/templates/create_foreign_key.tmpl b/pkg/writers/pgsql/templates/create_foreign_key.tmpl index 34e47dc..3fa2759 100644 --- a/pkg/writers/pgsql/templates/create_foreign_key.tmpl +++ b/pkg/writers/pgsql/templates/create_foreign_key.tmpl @@ -7,4 +7,4 @@ ALTER TABLE {{qual_table .SchemaName .TableName}} REFERENCES {{qual_table .TargetSchema .TargetTable}} ({{.TargetColumns}}) ON DELETE {{.OnDelete}} ON UPDATE {{.OnUpdate}} - DEFERRABLE; \ No newline at end of file + {{.Deferral}}; \ No newline at end of file diff --git a/pkg/writers/pgsql/templates/create_foreign_key_with_check.tmpl b/pkg/writers/pgsql/templates/create_foreign_key_with_check.tmpl index 8dbfcbe..20c1e30 100644 --- a/pkg/writers/pgsql/templates/create_foreign_key_with_check.tmpl +++ b/pkg/writers/pgsql/templates/create_foreign_key_with_check.tmpl @@ -11,8 +11,8 @@ BEGIN FOREIGN KEY ({{.SourceColumns}}) REFERENCES {{qual_table .TargetSchema .TargetTable}} ({{.TargetColumns}}) ON DELETE {{.OnDelete}} - ON UPDATE {{.OnUpdate}}{{if .Deferrable}} - DEFERRABLE{{end}}; + ON UPDATE {{.OnUpdate}}{{if .Deferral}} + {{.Deferral}}{{end}}; END IF; END; $$; \ No newline at end of file diff --git a/pkg/writers/pgsql/templates/create_unique_constraint.tmpl b/pkg/writers/pgsql/templates/create_unique_constraint.tmpl index 96684b7..7521fc2 100644 --- a/pkg/writers/pgsql/templates/create_unique_constraint.tmpl +++ b/pkg/writers/pgsql/templates/create_unique_constraint.tmpl @@ -6,7 +6,7 @@ BEGIN AND table_name = '{{.TableName}}' AND constraint_name = '{{.ConstraintName}}' ) THEN - ALTER TABLE {{qual_table .SchemaName .TableName}} ADD CONSTRAINT {{quote_ident .ConstraintName}} UNIQUE ({{.Columns}}); + ALTER TABLE {{qual_table .SchemaName .TableName}} ADD CONSTRAINT {{quote_ident .ConstraintName}} UNIQUE {{if .UsingIndex}}USING INDEX {{quote_ident .UsingIndex}}{{else}}({{.Columns}}){{end}}; END IF; END; -$$; \ No newline at end of file +$$; diff --git a/pkg/writers/pgsql/writer.go b/pkg/writers/pgsql/writer.go index a9d2ba6..103dd28 100644 --- a/pkg/writers/pgsql/writer.go +++ b/pkg/writers/pgsql/writer.go @@ -345,6 +345,7 @@ func (w *Writer) GenerateSchemaStatements(schema *models.Schema) ([]string, erro TableName: table.Name, ConstraintName: constraint.Name, Columns: strings.Join(constraint.Columns, ", "), + UsingIndex: adoptableUniqueIndex(table, constraint), } stmt, err := w.executor.ExecuteCreateUniqueConstraint(data) @@ -411,7 +412,7 @@ func (w *Writer) GenerateSchemaStatements(schema *models.Schema) ([]string, erro TargetColumns: strings.Join(constraint.ReferencedColumns, ", "), OnDelete: onDelete, OnUpdate: onUpdate, - Deferrable: false, + Deferral: constraint.DeferralClause(), } stmt, err := w.executor.ExecuteCreateForeignKeyWithCheck(data) @@ -1150,7 +1151,7 @@ func (w *Writer) writeIndexes(schema *models.Schema) error { if indexName == "" { indexType := "idx" if index.Unique { - indexType = "uidx" + indexType = "ukey" } columnSuffix := strings.Join(index.Columns, "_") indexName = fmt.Sprintf("%s_%s_%s", indexType, table.SQLName(), strings.ToLower(columnSuffix)) @@ -1236,6 +1237,7 @@ func (w *Writer) writeUniqueConstraints(schema *models.Schema) error { TableName: table.Name, ConstraintName: constraint.Name, Columns: strings.Join(columnExprs, ", "), + UsingIndex: adoptableUniqueIndex(table, constraint), }) if err != nil { return fmt.Errorf("failed to generate unique constraint: %w", err) @@ -1380,7 +1382,7 @@ func (w *Writer) writeForeignKeys(schema *models.Schema) error { TargetColumns: strings.Join(targetColumns, ", "), OnDelete: onDelete, OnUpdate: onUpdate, - Deferrable: true, + Deferral: "DEFERRABLE", } sql, err := w.executor.ExecuteCreateForeignKeyWithCheck(data) if err != nil { @@ -1459,7 +1461,7 @@ func (w *Writer) writeForeignKeys(schema *models.Schema) error { TargetColumns: strings.Join(targetColumns, ", "), OnDelete: onDelete, OnUpdate: onUpdate, - Deferrable: false, + Deferral: constraint.DeferralClause(), } sql, err := w.executor.ExecuteCreateForeignKeyWithCheck(data) if err != nil {