From 8708552d10b15a47a7657beafaee285dad46fcc5 Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Sat, 3 Aug 2024 16:42:21 +0200 Subject: [PATCH] refactor: remove AlterTableQuery bloat and fix test errors - Do not implement AppendQuery on Operation level. This is a leaky abstraction as queries are dialect-specific and migrate package should not be concerned with how they are constructed. - AlterTableQuery also is an unnecessary abstraction. Now pgdialect will just build a simple string-query for each Operation. - Moved operations.go to migrate/ package and deleted alt/ package. - Minor clean-ups and documentation. testChangeColumnType is commented out because the implementation is missing. --- dialect/pgdialect/alter_table.go | 324 +++++++++++-------------------- internal/dbtest/migrate_test.go | 8 +- migrate/diff.go | 67 ++++--- migrate/{alt => }/operations.go | 33 +--- migrate/sqlschema/migrator.go | 21 +- migrate/sqlschema/state.go | 38 ++-- schema/sqlfmt.go | 2 +- 7 files changed, 187 insertions(+), 306 deletions(-) rename migrate/{alt => }/operations.go (80%) diff --git a/dialect/pgdialect/alter_table.go b/dialect/pgdialect/alter_table.go index 15034d042..5170ba316 100644 --- a/dialect/pgdialect/alter_table.go +++ b/dialect/pgdialect/alter_table.go @@ -2,271 +2,175 @@ package pgdialect import ( "context" - "errors" "fmt" "github.com/uptrace/bun" - "github.com/uptrace/bun/migrate/alt" + "github.com/uptrace/bun/internal" + "github.com/uptrace/bun/migrate" "github.com/uptrace/bun/migrate/sqlschema" "github.com/uptrace/bun/schema" ) func (d *Dialect) Migrator(db *bun.DB) sqlschema.Migrator { - return &Migrator{db: db, BaseMigrator: sqlschema.NewBaseMigrator(db)} + return &migrator{db: db, BaseMigrator: sqlschema.NewBaseMigrator(db)} } -type Migrator struct { +type migrator struct { *sqlschema.BaseMigrator db *bun.DB } -var _ sqlschema.Migrator = (*Migrator)(nil) +var _ sqlschema.Migrator = (*migrator)(nil) -func (m *Migrator) execRaw(ctx context.Context, q *bun.RawQuery) error { - if _, err := q.Exec(ctx); err != nil { - return err - } - return nil -} - -func (m *Migrator) RenameTable(ctx context.Context, oldName, newName string) error { - q := m.db.NewRaw("ALTER TABLE ? RENAME TO ?", bun.Ident(oldName), bun.Ident(newName)) - return m.execRaw(ctx, q) -} - -func (m *Migrator) AddContraint(ctx context.Context, fk sqlschema.FK, name string) error { - q := m.db.NewRaw( - "ALTER TABLE ?.? ADD CONSTRAINT ? FOREIGN KEY (?) REFERENCES ?.? (?)", - bun.Safe(fk.From.Schema), bun.Safe(fk.From.Table), bun.Safe(name), - bun.Safe(fk.From.Column.String()), - bun.Safe(fk.To.Schema), bun.Safe(fk.To.Table), - bun.Safe(fk.To.Column.String()), - ) - return m.execRaw(ctx, q) -} - -func (m *Migrator) DropContraint(ctx context.Context, schema, table, name string) error { - q := m.db.NewRaw( - "ALTER TABLE ?.? DROP CONSTRAINT ?", - bun.Ident(schema), bun.Ident(table), bun.Ident(name), - ) - return m.execRaw(ctx, q) -} - -func (m *Migrator) RenameConstraint(ctx context.Context, schema, table, oldName, newName string) error { - q := m.db.NewRaw( - "ALTER TABLE ?.? RENAME CONSTRAINT ? TO ?", - bun.Ident(schema), bun.Ident(table), bun.Ident(oldName), bun.Ident(newName), - ) - return m.execRaw(ctx, q) -} - -func (m *Migrator) RenameColumn(ctx context.Context, schema, table, oldName, newName string) error { - q := m.db.NewRaw( - "ALTER TABLE ?.? RENAME COLUMN ? TO ?", - bun.Ident(schema), bun.Ident(table), bun.Ident(oldName), bun.Ident(newName), - ) - return m.execRaw(ctx, q) -} - -// ------------- - -func (m *Migrator) Apply(ctx context.Context, changes ...sqlschema.Operation) error { +func (m *migrator) Apply(ctx context.Context, changes ...sqlschema.Operation) error { if len(changes) == 0 { return nil } + var conn bun.IConn + var err error - queries, err := m.buildQueries(changes...) - if err != nil { - return fmt.Errorf("apply database schema changes: %w", err) - } - - for _, query := range queries { - var b []byte - if b, err = query.AppendQuery(m.db.Formatter(), b); err != nil { - return err - } - m.execRaw(ctx, m.db.NewRaw(string(b))) + if conn, err = m.db.Conn(ctx); err != nil { + return err } - return nil -} - -// buildQueries combines schema changes to a number of ALTER TABLE queries. -func (m *Migrator) buildQueries(changes ...sqlschema.Operation) ([]*AlterTableQuery, error) { - var queries []*AlterTableQuery + fmter := m.db.Formatter() + for _, change := range changes { + var b []byte // TODO(dyma): call db.MakeQueryBytes - chain := func(change sqlschema.Operation) error { - for _, query := range queries { - if err := query.Chain(change); err != errCannotChain { - return err // either nil (successful) or non-nil (failed) + switch change := change.(type) { + case *migrate.CreateTable: + err = m.CreateTable(ctx, change.Model) + if err != nil { + return fmt.Errorf("apply changes: create table %s: %w", change.FQN(), err) + } + continue + case *migrate.DropTable: + err = m.DropTable(ctx, change.Schema, change.Name) + if err != nil { + return fmt.Errorf("apply changes: drop table %s: %w", change.FQN(), err) } + continue + case *migrate.RenameTable: + b, err = m.renameTable(fmter, b, change) + case *migrate.RenameColumn: + b, err = m.renameColumn(fmter, b, change) + case *migrate.DropConstraint: + b, err = m.dropContraint(fmter, b, change) + case *migrate.AddForeignKey: + b, err = m.addForeignKey(fmter, b, change) + case *migrate.RenameConstraint: + b, err = m.renameConstraint(fmter, b, change) + default: + return fmt.Errorf("apply changes: unknown operation %T", change) } - - // Create a new query for this change, since it cannot be chained to any of the existing ones. - q, err := newAlterTableQuery(change) if err != nil { - return err + return fmt.Errorf("apply changes: %w", err) } - queries = append(queries, q.Sep()) - return nil - } - for _, change := range changes { - if err := chain(change); err != nil { - return nil, err + query := internal.String(b) + // log.Println("exec query: " + query) + if _, err = conn.ExecContext(ctx, query); err != nil { + return fmt.Errorf("apply changes: %w", err) } } - return queries, nil -} - -type AlterTableQuery struct { - FQN schema.FQN - - RenameTable sqlschema.Operation - RenameColumn sqlschema.Operation - RenameConstraint sqlschema.Operation - Actions Actions - - separate bool + return nil } -type Actions []*Action - -var _ schema.QueryAppender = (*Actions)(nil) - -type Action struct { - AddColumn sqlschema.Operation - DropColumn sqlschema.Operation - AlterColumn sqlschema.Operation - AlterType sqlschema.Operation - SetDefault sqlschema.Operation - DropDefault sqlschema.Operation - SetNotNull sqlschema.Operation - DropNotNull sqlschema.Operation - AddGenerated sqlschema.Operation - AddConstraint sqlschema.Operation - DropConstraint sqlschema.Operation - Custom sqlschema.Operation +func (m *migrator) renameTable(fmter schema.Formatter, b []byte, rename *migrate.RenameTable) (_ []byte, err error) { + b = append(b, "ALTER TABLE "...) + fqn := rename.FQN() + if b, err = fqn.AppendQuery(fmter, b); err != nil { + return b, err + } + b = append(b, " RENAME TO "...) + if b, err = bun.Ident(rename.NewName).AppendQuery(fmter, b); err != nil { + return b, err + } + return b, nil } -var _ schema.QueryAppender = (*Action)(nil) - -func newAlterTableQuery(op sqlschema.Operation) (*AlterTableQuery, error) { - q := AlterTableQuery{ - FQN: op.FQN(), +func (m *migrator) renameColumn(fmter schema.Formatter, b []byte, rename *migrate.RenameColumn) (_ []byte, err error) { + b = append(b, "ALTER TABLE "...) + fqn := rename.FQN() + if b, err = fqn.AppendQuery(fmter, b); err != nil { + return b, err } - switch op.(type) { - case *alt.RenameTable: - q.RenameTable = op - case *alt.RenameColumn: - q.RenameColumn = op - case *alt.RenameConstraint: - q.RenameConstraint = op - default: - q.Actions = append(q.Actions, newAction(op)) + + b = append(b, " RENAME COLUMN "...) + if b, err = bun.Ident(rename.OldName).AppendQuery(fmter, b); err != nil { + return b, err } - return &q, nil -} -func newAction(op sqlschema.Operation) *Action { - var a Action - return &a + b = append(b, " TO "...) + if b, err = bun.Ident(rename.NewName).AppendQuery(fmter, b); err != nil { + return b, err + } + return b, nil } -// errCannotChain is a sentinel error. To apply the change, callers should -// create a new AlterTableQuery instead and include it there. -var errCannotChain = errors.New("cannot chain change to the current query") +func (m *migrator) renameConstraint(fmter schema.Formatter, b []byte, rename *migrate.RenameConstraint) (_ []byte, err error) { + b = append(b, "ALTER TABLE "...) + fqn := rename.FQN() + if b, err = fqn.AppendQuery(fmter, b); err != nil { + return b, err + } -func (q *AlterTableQuery) Chain(op sqlschema.Operation) error { - if op.FQN() != q.FQN { - return errCannotChain + b = append(b, " RENAME CONSTRAINT "...) + if b, err = bun.Ident(rename.OldName).AppendQuery(fmter, b); err != nil { + return b, err } - switch op.(type) { - default: - return fmt.Errorf("unsupported operation %T", op) + b = append(b, " TO "...) + if b, err = bun.Ident(rename.NewName).AppendQuery(fmter, b); err != nil { + return b, err } + return b, nil } -func (q *AlterTableQuery) isEmpty() bool { - return q.RenameTable == nil && q.RenameColumn == nil && q.RenameConstraint == nil && len(q.Actions) == 0 -} +func (m *migrator) dropContraint(fmter schema.Formatter, b []byte, drop *migrate.DropConstraint) (_ []byte, err error) { + b = append(b, "ALTER TABLE "...) + fqn := drop.FQN() + if b, err = fqn.AppendQuery(fmter, b); err != nil { + return b, err + } -// Sep appends a ";" separator at the end of the query. -func (q *AlterTableQuery) Sep() *AlterTableQuery { - q.separate = true - return q + b = append(b, " DROP CONSTRAINT "...) + if b, err = bun.Ident(drop.ConstraintName).AppendQuery(fmter, b); err != nil { + return b, err + } + return b, nil } -func (q *AlterTableQuery) AppendQuery(fmter schema.Formatter, b []byte) (_ []byte, err error) { - var op schema.QueryAppender - switch true { - case q.RenameTable != nil: - op = q.RenameTable - case q.RenameColumn != nil: - op = q.RenameColumn - case q.RenameConstraint != nil: - op = q.RenameConstraint - case len(q.Actions) > 0: - op = q.Actions - default: - return b, nil - } +func (m *migrator) addForeignKey(fmter schema.Formatter, b []byte, add *migrate.AddForeignKey) (_ []byte, err error) { b = append(b, "ALTER TABLE "...) - b, _ = q.FQN.AppendQuery(fmter, b) - b = append(b, " "...) - if b, err = op.AppendQuery(fmter, b); err != nil { + fqn := add.FQN() + if b, err = fqn.AppendQuery(fmter, b); err != nil { return b, err } - if q.separate { - b = append(b, ";"...) + b = append(b, " ADD CONSTRAINT "...) + if b, err = bun.Ident(add.ConstraintName).AppendQuery(fmter, b); err != nil { + return b, err } - return b, nil -} -func (actions Actions) AppendQuery(fmter schema.Formatter, b []byte) (_ []byte, err error) { - for i, a := range actions { - if i > 0 { - b = append(b, ", "...) - } - b, err = a.AppendQuery(fmter, b) - if err != nil { - return b, err - } + b = append(b, " FOREIGN KEY ("...) + if b, err = add.FK.From.Column.Safe().AppendQuery(fmter, b); err != nil { + return b, err + } + b = append(b, ") "...) + + other := schema.FQN{Schema: add.FK.To.Schema, Table: add.FK.To.Table} + b = append(b, " REFERENCES "...) + if b, err = other.AppendQuery(fmter, b); err != nil { + return b, err } - return b, nil -} -func (a *Action) AppendQuery(fmter schema.Formatter, b []byte) ([]byte, error) { - var op schema.QueryAppender - switch true { - case a.AddColumn != nil: - op = a.AddColumn - case a.DropColumn != nil: - op = a.DropColumn - case a.AlterColumn != nil: - op = a.AlterColumn - case a.AlterType != nil: - op = a.AlterType - case a.SetDefault != nil: - op = a.SetDefault - case a.DropDefault != nil: - op = a.DropDefault - case a.SetNotNull != nil: - op = a.SetNotNull - case a.DropNotNull != nil: - op = a.DropNotNull - case a.AddGenerated != nil: - op = a.AddGenerated - case a.AddConstraint != nil: - op = a.AddConstraint - case a.DropConstraint != nil: - op = a.DropConstraint - default: - return b, nil + b = append(b, " ("...) + if b, err = add.FK.To.Column.Safe().AppendQuery(fmter, b); err != nil { + return b, err } - return op.AppendQuery(fmter, b) + b = append(b, ")"...) + + return b, nil } diff --git a/internal/dbtest/migrate_test.go b/internal/dbtest/migrate_test.go index 037ef32dc..e362db7d1 100644 --- a/internal/dbtest/migrate_test.go +++ b/internal/dbtest/migrate_test.go @@ -201,9 +201,9 @@ func TestAutoMigrator_Run(t *testing.T) { }{ {testRenameTable}, {testRenamedColumns}, - // {testCreateDropTable}, - // {testAlterForeignKeys}, - // {testCustomFKNameFunc}, + {testCreateDropTable}, + {testAlterForeignKeys}, + {testCustomFKNameFunc}, {testForceRenameFK}, {testRenameColumnRenamesFK}, // {testChangeColumnType}, @@ -470,7 +470,7 @@ func testCustomFKNameFunc(t *testing.T, db *bun.DB) { From: sqlschema.C(db.Dialect().DefaultSchema(), "columns", "attrelid"), To: sqlschema.C(db.Dialect().DefaultSchema(), "tables", "oid"), }] - require.Equal(t, fkName, "test_fkey") + require.Equal(t, "test_fkey", fkName) } func testRenamedColumns(t *testing.T, db *bun.DB) { diff --git a/migrate/diff.go b/migrate/diff.go index 4c875975c..3406bb76c 100644 --- a/migrate/diff.go +++ b/migrate/diff.go @@ -7,17 +7,16 @@ import ( "strings" "github.com/uptrace/bun" - "github.com/uptrace/bun/migrate/alt" "github.com/uptrace/bun/migrate/sqlschema" ) // changeset is a set of changes to the database definition. type changeset struct { - operations []alt.Operation + operations []Operation } // Add new operations to the changeset. -func (c *changeset) Add(op ...alt.Operation) { +func (c *changeset) Add(op ...Operation) { c.operations = append(c.operations, op...) } @@ -57,17 +56,17 @@ func (c *changeset) ResolveDependencies() error { visited ) - var resolved []alt.Operation - var visit func(op alt.Operation) error + var resolved []Operation + var visit func(op Operation) error - var nextOp alt.Operation + var nextOp Operation var next func() bool - status := make(map[alt.Operation]int, len(c.operations)) + status := make(map[Operation]int, len(c.operations)) for _, op := range c.operations { status[op] = unvisited } - + next = func() bool { for op, s := range status { if s == unvisited { @@ -80,7 +79,7 @@ func (c *changeset) ResolveDependencies() error { // visit iterates over c.operations until it finds all operations that depend on the current one // or runs into cirtular dependency, in which case it will return an error. - visit = func(op alt.Operation) error { + visit = func(op Operation) error { switch status[op] { case visited: return nil @@ -93,7 +92,7 @@ func (c *changeset) ResolveDependencies() error { for _, another := range c.operations { if dop, hasDeps := another.(interface { - DependsOn(alt.Operation) bool + DependsOn(Operation) bool }); another == op || !hasDeps || !dop.DependsOn(op) { continue } @@ -105,7 +104,7 @@ func (c *changeset) ResolveDependencies() error { status[op] = visited // Any dependent nodes would've already been added to the list by now, so we prepend. - resolved = append([]alt.Operation{op}, resolved...) + resolved = append([]Operation{op}, resolved...) return nil } @@ -184,7 +183,7 @@ func (d *detector) Diff() *changeset { targetTables := newTableSet(d.target.Tables...) currentTables := newTableSet(d.current.Tables...) // keeps state (which models still need to be checked) - // These table sets record "updates" to the targetTables set. + // These table-sets record changes to the targetTables set. created := newTableSet() renamed := newTableSet() @@ -194,15 +193,17 @@ AddedLoop: removedTables := currentTables.Sub(targetTables) for _, removed := range removedTables.Values() { if d.canRename(removed, added) { - d.changes.Add(&alt.RenameTable{ + d.changes.Add(&RenameTable{ Schema: removed.Schema, OldName: removed.Name, NewName: added.Name, }) + // Here we do not check for created / dropped columns,as well as column type changes, + // because it is only possible to detect a renamed table if its signature (see state.go) did not change. d.detectRenamedColumns(removed, added) - // Update referenced table in all related FKs + // Update referenced table in all related FKs. if d.detectRenamedFKs { d.refMap.UpdateT(removed.T(), added.T()) } @@ -215,7 +216,7 @@ AddedLoop: } } // If a new table did not appear because of the rename operation, then it must've been created. - d.changes.Add(&alt.CreateTable{ + d.changes.Add(&CreateTable{ Schema: added.Schema, Name: added.Name, Model: added.Model, @@ -226,19 +227,20 @@ AddedLoop: // Tables that aren't present anymore and weren't renamed or left untouched were deleted. dropped := currentTables.Sub(targetTables) for _, t := range dropped.Values() { - d.changes.Add(&alt.DropTable{ + d.changes.Add(&DropTable{ Schema: t.Schema, Name: t.Name, }) } - // Detect changes in existing tables that weren't renamed + // Detect changes in existing tables that weren't renamed. + // // TODO: here having State.Tables be a map[string]Table would be much more convenient. // Then we can alse retire tableSet, or at least simplify it to a certain extent. curEx := currentTables.Sub(dropped) tarEx := targetTables.Sub(created).Sub(renamed) for _, target := range tarEx.Values() { - // This step is redundant if we have map[string]Table + // TODO(dyma): step is redundant if we have map[string]Table var current sqlschema.Table for _, cur := range curEx.Values() { if cur.Name == target.Name { @@ -259,13 +261,13 @@ AddedLoop: // Add RenameFK migrations for updated FKs. for old, renamed := range d.refMap.Updated() { newName := d.fkNameFunc(renamed) - d.changes.Add(&alt.RenameConstraint{ + d.changes.Add(&RenameConstraint{ FK: renamed, // TODO: make sure this is applied after the table/columns are renamed OldName: d.current.FKs[old], - NewName: d.fkNameFunc(renamed), + NewName: newName, }) - // Here we can add this fk to "current.FKs" to prevent it from firing in the next 2 for-loops. + // Add this FK to currentFKs to prevent it from firing in the two loops below. currentFKs[renamed] = newName delete(currentFKs, old) } @@ -274,7 +276,7 @@ AddedLoop: // Add AddFK migrations for newly added FKs. for fk := range d.target.FKs { if _, ok := currentFKs[fk]; !ok { - d.changes.Add(&alt.AddForeignKey{ + d.changes.Add(&AddForeignKey{ FK: fk, ConstraintName: d.fkNameFunc(fk), }) @@ -284,7 +286,7 @@ AddedLoop: // Add DropFK migrations for removed FKs. for fk, fkName := range currentFKs { if _, ok := d.target.FKs[fk]; !ok { - d.changes.Add(&alt.DropConstraint{ + d.changes.Add(&DropConstraint{ FK: fk, ConstraintName: fkName, }) @@ -309,7 +311,7 @@ func (d *detector) detectRenamedColumns(current, added sqlschema.Table) { if aCol != cCol { continue } - d.changes.Add(&alt.RenameColumn{ + d.changes.Add(&RenameColumn{ Schema: added.Schema, Table: added.Name, OldName: cName, @@ -329,8 +331,8 @@ type tableSet struct { underlying map[string]sqlschema.Table } -func newTableSet(initial ...sqlschema.Table) tableSet { - set := tableSet{ +func newTableSet(initial ...sqlschema.Table) *tableSet { + set := &tableSet{ underlying: make(map[string]sqlschema.Table), } for _, t := range initial { @@ -339,22 +341,22 @@ func newTableSet(initial ...sqlschema.Table) tableSet { return set } -func (set tableSet) Add(t sqlschema.Table) { +func (set *tableSet) Add(t sqlschema.Table) { set.underlying[t.Name] = t } -func (set tableSet) Remove(s string) { +func (set *tableSet) Remove(s string) { delete(set.underlying, s) } -func (set tableSet) Values() (tables []sqlschema.Table) { +func (set *tableSet) Values() (tables []sqlschema.Table) { for _, t := range set.underlying { tables = append(tables, t) } return } -func (set tableSet) Sub(other tableSet) tableSet { +func (set *tableSet) Sub(other *tableSet) *tableSet { res := set.clone() for v := range other.underlying { if _, ok := set.underlying[v]; ok { @@ -364,7 +366,7 @@ func (set tableSet) Sub(other tableSet) tableSet { return res } -func (set tableSet) clone() tableSet { +func (set *tableSet) clone() *tableSet { res := newTableSet() for _, t := range set.underlying { res.Add(t) @@ -372,7 +374,8 @@ func (set tableSet) clone() tableSet { return res } -func (set tableSet) String() string { +// String is a debug helper to get a list of table names in the set. +func (set *tableSet) String() string { var s strings.Builder for k := range set.underlying { if s.Len() > 0 { diff --git a/migrate/alt/operations.go b/migrate/operations.go similarity index 80% rename from migrate/alt/operations.go rename to migrate/operations.go index f7f1a8873..14671f86a 100644 --- a/migrate/alt/operations.go +++ b/migrate/operations.go @@ -1,7 +1,6 @@ -package alt +package migrate import ( - "github.com/uptrace/bun" "github.com/uptrace/bun/migrate/sqlschema" "github.com/uptrace/bun/schema" ) @@ -51,6 +50,7 @@ func (op *DropTable) FQN() schema.FQN { func (op *DropTable) DependsOn(another Operation) bool { d, ok := another.(*DropConstraint) + // return ok && ((d.FK.From.Schema == op.Schema && d.FK.From.Table == op.Name) || (d.FK.To.Schema == op.Schema && d.FK.To.Table == op.Name)) } @@ -80,10 +80,6 @@ func (op *RenameTable) FQN() schema.FQN { } } -func (op *RenameTable) AppendQuery(fmter schema.Formatter, b []byte) ([]byte, error) { - return fmter.AppendQuery(b, "RENAME TO ?", bun.Ident(op.NewName)), nil -} - func (op *RenameTable) GetReverse() Operation { return &RenameTable{ Schema: op.Schema, @@ -110,10 +106,6 @@ func (op *RenameColumn) FQN() schema.FQN { } } -func (op *RenameColumn) AppendQuery(fmter schema.Formatter, b []byte) ([]byte, error) { - return fmter.AppendQuery(b, "RENAME COLUMN ? TO ?", bun.Ident(op.OldName), bun.Ident(op.NewName)), nil -} - func (op *RenameColumn) GetReverse() Operation { return &RenameColumn{ Schema: op.Schema, @@ -150,10 +142,6 @@ func (op *RenameConstraint) DependsOn(another Operation) bool { return ok && rt.Schema == op.FK.From.Schema && rt.NewName == op.FK.From.Table } -func (op *RenameConstraint) AppendQuery(fmter schema.Formatter, b []byte) ([]byte, error) { - return fmter.AppendQuery(b, "RENAME CONSTRAINT ? TO ?", bun.Ident(op.OldName), bun.Ident(op.NewName)), nil -} - func (op *RenameConstraint) GetReverse() Operation { return &RenameConstraint{ FK: op.FK, @@ -188,19 +176,6 @@ func (op *AddForeignKey) DependsOn(another Operation) bool { return false } -func (op *AddForeignKey) AppendQuery(fmter schema.Formatter, b []byte) ([]byte, error) { - fqn := schema.FQN{ - Schema: op.FK.To.Schema, - Table: op.FK.To.Table, - } - b = fmter.AppendQuery(b, - "ADD CONSTRAINT ? FOREIGN KEY (?) REFERENCES ", - bun.Ident(op.ConstraintName), bun.Safe(op.FK.From.Column), - ) - b, _ = fqn.AppendQuery(fmter, b) - return fmter.AppendQuery(b, " (?)", bun.Ident(op.FK.To.Column)), nil -} - func (op *AddForeignKey) GetReverse() Operation { return &DropConstraint{ FK: op.FK, @@ -224,10 +199,6 @@ func (op *DropConstraint) FQN() schema.FQN { } } -func (op *DropConstraint) AppendQuery(fmter schema.Formatter, b []byte) ([]byte, error) { - return fmter.AppendQuery(b, "DROP CONSTRAINT ?", bun.Ident(op.ConstraintName)), nil -} - func (op *DropConstraint) GetReverse() Operation { return &AddForeignKey{ FK: op.FK, diff --git a/migrate/sqlschema/migrator.go b/migrate/sqlschema/migrator.go index 3bdeb7e08..6087a8448 100644 --- a/migrate/sqlschema/migrator.go +++ b/migrate/sqlschema/migrator.go @@ -13,24 +13,11 @@ type MigratorDialect interface { Migrator(*bun.DB) Migrator } -type Operation interface { - schema.QueryAppender - FQN() schema.FQN -} - type Migrator interface { Apply(ctx context.Context, changes ...Operation) error - - RenameTable(ctx context.Context, oldName, newName string) error - CreateTable(ctx context.Context, model interface{}) error - DropTable(ctx context.Context, schema, table string) error - AddContraint(ctx context.Context, fk FK, name string) error - DropContraint(ctx context.Context, schema, table, name string) error - RenameConstraint(ctx context.Context, schema, table, oldName, newName string) error - RenameColumn(ctx context.Context, schema, table, oldName, newName string) error } -// Migrator is a dialect-agnostic wrapper for sqlschema.Dialect +// migrator is a dialect-agnostic wrapper for sqlschema.MigratorDialect. type migrator struct { Migrator } @@ -69,3 +56,9 @@ func (m *BaseMigrator) DropTable(ctx context.Context, schema, name string) error } return nil } + +// Operation is a helper interface each migrate.Operation must implement +// so an not to handle this in every dialect separately. +type Operation interface { + FQN() schema.FQN +} diff --git a/migrate/sqlschema/state.go b/migrate/sqlschema/state.go index 553634d90..40fda8320 100644 --- a/migrate/sqlschema/state.go +++ b/migrate/sqlschema/state.go @@ -1,6 +1,10 @@ package sqlschema -import "strings" +import ( + "strings" + + "github.com/uptrace/bun/schema" +) type State struct { Tables []Table @@ -14,6 +18,7 @@ type Table struct { Columns map[string]Column } +// T returns a fully-qualified name object for the table. func (t *Table) T() tFQN { return T(t.Schema, t.Name) } @@ -78,6 +83,7 @@ type tFQN struct { Table string } +// T creates a fully-qualified table name object. func T(schema, table string) tFQN { return tFQN{Schema: schema, Table: table} } // cFQN is a fully-qualified column name. @@ -86,6 +92,7 @@ type cFQN struct { Column composite } +// C creates a fully-qualified column name object. func C(schema, table string, columns ...string) cFQN { return cFQN{tFQN: T(schema, table), Column: newComposite(columns...)} } @@ -96,7 +103,7 @@ func (c cFQN) T() tFQN { } // composite is a hashable representation of []string used to define FKs that depend on multiple columns. -// Although having duplicated column references in a FK is illegal, composite neither validate nor enforce this constraint on the caller. +// Although having duplicated column references in a FK is illegal, composite neither validates nor enforces this constraint on the caller. type composite string // newComposite creates a composite column from a slice of column names. @@ -108,6 +115,10 @@ func (c composite) String() string { return string(c) } +func (c composite) Safe() schema.Safe { + return schema.Safe(c) +} + // Split returns a slice of column names that make up the composite. func (c composite) Split() []string { return strings.Split(c.String(), ",") @@ -151,14 +162,14 @@ func (c composite) Replace(oldColumn, newColumn string) composite { // // fk := FK{ // From: C("a", "b", "c_1", "c_2"), // supports multicolumn FKs -// To: C("w", "x", "y_1", "y_2") +// To: C("w", "x", "y_1", "y_2") // } type FK struct { From cFQN // From is the referencing column. To cFQN // To is the referenced column. } -// DependsT checks if either part of the FK's definition mentions T +// dependsT checks if either part of the FK's definition mentions T // and returns the columns that belong to T. Notice that *C allows modifying the column's FQN. // // Example: @@ -168,7 +179,7 @@ type FK struct { // To: C("x", "y", "z"), // } // depends on T("a", "b") and T("x", "y") -func (fk *FK) DependsT(t tFQN) (ok bool, cols []*cFQN) { +func (fk *FK) dependsT(t tFQN) (ok bool, cols []*cFQN) { if c := &fk.From; c.T() == t { ok = true cols = append(cols, c) @@ -183,7 +194,7 @@ func (fk *FK) DependsT(t tFQN) (ok bool, cols []*cFQN) { return } -// DependsC checks if the FK definition mentions C and returns a modifiable FQN of the matching column. +// dependsC checks if the FK definition mentions C and returns a modifiable FQN of the matching column. // // Example: // @@ -192,7 +203,7 @@ func (fk *FK) DependsT(t tFQN) (ok bool, cols []*cFQN) { // To: C("w", "x", "y_1", "y_2"), // } // depends on C("a", "b", "c_1"), C("a", "b", "c_2"), C("w", "x", "y_1"), and C("w", "x", "y_2") -func (fk *FK) DependsC(c cFQN) (bool, *cFQN) { +func (fk *FK) dependsC(c cFQN) (bool, *cFQN) { switch { case fk.From.Column.Contains(c.Column): return true, &fk.From @@ -208,8 +219,7 @@ func (fk *FK) DependsC(c cFQN) (bool, *cFQN) { // // Note: this is only important/necessary if we want to rename FKs instead of re-creating them. // Most of the time it wouldn't make a difference, but there may be cases in which re-creating FKs could be costly -// and renaming them would be preferred. For that we could provided an options like WithRenameFKs(true) and -// WithRenameFKFunc(func(sqlschema.FK) string) to allow customizing the FK naming convention. +// and renaming them would be preferred. type RefMap map[FK]*FK // deleted is a special value that RefMap uses to denote a deleted FK constraint. @@ -229,7 +239,7 @@ func NewRefMap(fks ...FK) RefMap { // Returns the number of updated entries. func (r RefMap) UpdateT(oldT, newT tFQN) (n int) { for _, fk := range r { - ok, cols := fk.DependsT(oldT) + ok, cols := fk.dependsT(oldT) if !ok { continue } @@ -246,9 +256,9 @@ func (r RefMap) UpdateT(oldT, newT tFQN) (n int) { // and so, only the column-name part of the FQN can be updated. Returns the number of updated entries. func (r RefMap) UpdateC(oldC cFQN, newColumn string) (n int) { for _, fk := range r { - if ok, col := fk.DependsC(oldC); ok { + if ok, col := fk.dependsC(oldC); ok { oldColumns := oldC.Column.Split() - // UpdateC can only update 1 column at a time. + // updateC will only update 1 column per invocation. col.Column = col.Column.Replace(oldColumns[0], newColumn) n++ } @@ -260,7 +270,7 @@ func (r RefMap) UpdateC(oldC cFQN, newColumn string) (n int) { // Returns the number of deleted entries. func (r RefMap) DeleteT(t tFQN) (n int) { for old, fk := range r { - if ok, _ := fk.DependsT(t); ok { + if ok, _ := fk.dependsT(t); ok { r[old] = &deleted n++ } @@ -272,7 +282,7 @@ func (r RefMap) DeleteT(t tFQN) (n int) { // Returns the number of deleted entries. func (r RefMap) DeleteC(c cFQN) (n int) { for old, fk := range r { - if ok, _ := fk.DependsC(c); ok { + if ok, _ := fk.dependsC(c); ok { r[old] = &deleted n++ } diff --git a/schema/sqlfmt.go b/schema/sqlfmt.go index 11eabb13b..5703c9694 100644 --- a/schema/sqlfmt.go +++ b/schema/sqlfmt.go @@ -39,7 +39,7 @@ func (s Name) AppendQuery(fmter Formatter, b []byte) ([]byte, error) { //------------------------------------------------------------------------------ -// FQN represents a fully qualified table name. +// FQN appends a fully-qualified table name. type FQN struct { Schema string Table string