From a8e5b3d1fee5b09c0d5e70f0bcefa8c9daf9370e Mon Sep 17 00:00:00 2001 From: dyma solovei Date: Sat, 9 Dec 2023 15:45:59 +0100 Subject: [PATCH] wip: resolve dependencies + refactor --- dialect/pgdialect/alter_table.go | 217 ++++++++++- internal/dbtest/db_test.go | 14 +- internal/dbtest/migrate_test.go | 616 ++++++++++++++++++------------- migrate/alt/operations.go | 263 +++++++++++++ migrate/auto.go | 597 ++---------------------------- migrate/diff.go | 390 +++++++++++++++++++ migrate/migrator.go | 4 +- migrate/sqlschema/inspector.go | 7 + migrate/sqlschema/migrator.go | 7 + schema/sqlfmt.go | 19 + schema/table.go | 2 + 11 files changed, 1280 insertions(+), 856 deletions(-) create mode 100644 migrate/alt/operations.go create mode 100644 migrate/diff.go diff --git a/dialect/pgdialect/alter_table.go b/dialect/pgdialect/alter_table.go index 71b090e46..15034d042 100644 --- a/dialect/pgdialect/alter_table.go +++ b/dialect/pgdialect/alter_table.go @@ -2,9 +2,13 @@ package pgdialect import ( "context" + "errors" + "fmt" "github.com/uptrace/bun" + "github.com/uptrace/bun/migrate/alt" "github.com/uptrace/bun/migrate/sqlschema" + "github.com/uptrace/bun/schema" ) func (d *Dialect) Migrator(db *bun.DB) sqlschema.Migrator { @@ -19,7 +23,7 @@ type Migrator struct { var _ sqlschema.Migrator = (*Migrator)(nil) -func (m *Migrator) exec(ctx context.Context, q *bun.RawQuery) error { +func (m *Migrator) execRaw(ctx context.Context, q *bun.RawQuery) error { if _, err := q.Exec(ctx); err != nil { return err } @@ -28,7 +32,7 @@ func (m *Migrator) exec(ctx context.Context, q *bun.RawQuery) error { 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.exec(ctx, q) + return m.execRaw(ctx, q) } func (m *Migrator) AddContraint(ctx context.Context, fk sqlschema.FK, name string) error { @@ -39,7 +43,7 @@ func (m *Migrator) AddContraint(ctx context.Context, fk sqlschema.FK, name strin bun.Safe(fk.To.Schema), bun.Safe(fk.To.Table), bun.Safe(fk.To.Column.String()), ) - return m.exec(ctx, q) + return m.execRaw(ctx, q) } func (m *Migrator) DropContraint(ctx context.Context, schema, table, name string) error { @@ -47,7 +51,7 @@ func (m *Migrator) DropContraint(ctx context.Context, schema, table, name string "ALTER TABLE ?.? DROP CONSTRAINT ?", bun.Ident(schema), bun.Ident(table), bun.Ident(name), ) - return m.exec(ctx, q) + return m.execRaw(ctx, q) } func (m *Migrator) RenameConstraint(ctx context.Context, schema, table, oldName, newName string) error { @@ -55,7 +59,7 @@ func (m *Migrator) RenameConstraint(ctx context.Context, schema, table, oldName, "ALTER TABLE ?.? RENAME CONSTRAINT ? TO ?", bun.Ident(schema), bun.Ident(table), bun.Ident(oldName), bun.Ident(newName), ) - return m.exec(ctx, q) + return m.execRaw(ctx, q) } func (m *Migrator) RenameColumn(ctx context.Context, schema, table, oldName, newName string) error { @@ -63,5 +67,206 @@ func (m *Migrator) RenameColumn(ctx context.Context, schema, table, oldName, new "ALTER TABLE ?.? RENAME COLUMN ? TO ?", bun.Ident(schema), bun.Ident(table), bun.Ident(oldName), bun.Ident(newName), ) - return m.exec(ctx, q) + return m.execRaw(ctx, q) +} + +// ------------- + +func (m *Migrator) Apply(ctx context.Context, changes ...sqlschema.Operation) error { + if len(changes) == 0 { + return nil + } + + 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))) + } + + 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 + + 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) + } + } + + // 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 + } + queries = append(queries, q.Sep()) + return nil + } + + for _, change := range changes { + if err := chain(change); err != nil { + return nil, err + } + } + return queries, nil +} + +type AlterTableQuery struct { + FQN schema.FQN + + RenameTable sqlschema.Operation + RenameColumn sqlschema.Operation + RenameConstraint sqlschema.Operation + Actions Actions + + separate bool +} + +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 +} + +var _ schema.QueryAppender = (*Action)(nil) + +func newAlterTableQuery(op sqlschema.Operation) (*AlterTableQuery, error) { + q := AlterTableQuery{ + FQN: op.FQN(), + } + 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)) + } + return &q, nil +} + +func newAction(op sqlschema.Operation) *Action { + var a Action + return &a +} + +// 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 (q *AlterTableQuery) Chain(op sqlschema.Operation) error { + if op.FQN() != q.FQN { + return errCannotChain + } + + switch op.(type) { + default: + return fmt.Errorf("unsupported operation %T", op) + } +} + +func (q *AlterTableQuery) isEmpty() bool { + return q.RenameTable == nil && q.RenameColumn == nil && q.RenameConstraint == nil && len(q.Actions) == 0 +} + +// Sep appends a ";" separator at the end of the query. +func (q *AlterTableQuery) Sep() *AlterTableQuery { + q.separate = true + return q +} + +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 + } + b = append(b, "ALTER TABLE "...) + b, _ = q.FQN.AppendQuery(fmter, b) + b = append(b, " "...) + if b, err = op.AppendQuery(fmter, b); err != nil { + return b, err + } + + if q.separate { + b = append(b, ";"...) + } + 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 + } + } + 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 + } + return op.AppendQuery(fmter, b) } diff --git a/internal/dbtest/db_test.go b/internal/dbtest/db_test.go index 7bbf5708a..a9a303bf6 100644 --- a/internal/dbtest/db_test.go +++ b/internal/dbtest/db_test.go @@ -45,13 +45,13 @@ const ( ) var allDBs = map[string]func(tb testing.TB) *bun.DB{ - pgName: pg, - pgxName: pgx, - mysql5Name: mysql5, - mysql8Name: mysql8, - mariadbName: mariadb, - sqliteName: sqlite, - mssql2019Name: mssql2019, + pgName: pg, + // pgxName: pgx, + // mysql5Name: mysql5, + // mysql8Name: mysql8, + // mariadbName: mariadb, + // sqliteName: sqlite, + // mssql2019Name: mssql2019, } var allDialects = []func() schema.Dialect{ diff --git a/internal/dbtest/migrate_test.go b/internal/dbtest/migrate_test.go index 0a2d60e15..037ef32dc 100644 --- a/internal/dbtest/migrate_test.go +++ b/internal/dbtest/migrate_test.go @@ -3,16 +3,15 @@ package dbtest_test import ( "context" "errors" - "sort" "strings" "testing" "time" "github.com/stretchr/testify/require" "github.com/uptrace/bun" + "github.com/uptrace/bun/dialect/sqltype" "github.com/uptrace/bun/migrate" "github.com/uptrace/bun/migrate/sqlschema" - "github.com/uptrace/bun/schema" ) const ( @@ -201,12 +200,13 @@ func TestAutoMigrator_Run(t *testing.T) { fn func(t *testing.T, db *bun.DB) }{ {testRenameTable}, - {testCreateDropTable}, - {testAlterForeignKeys}, - {testCustomFKNameFunc}, - {testForceRenameFK}, {testRenamedColumns}, + // {testCreateDropTable}, + // {testAlterForeignKeys}, + // {testCustomFKNameFunc}, + {testForceRenameFK}, {testRenameColumnRenamesFK}, + // {testChangeColumnType}, } testEachDB(t, func(t *testing.T, dbName string, db *bun.DB) { @@ -476,7 +476,8 @@ func testCustomFKNameFunc(t *testing.T, db *bun.DB) { func testRenamedColumns(t *testing.T, db *bun.DB) { // Database state type Original struct { - ID int64 `bun:",pk"` + bun.BaseModel `bun:"original"` + ID int64 `bun:",pk"` } type Model1 struct { @@ -507,8 +508,8 @@ func testRenamedColumns(t *testing.T, db *bun.DB) { ) mustDropTableOnCleanup(t, ctx, db, (*Renamed)(nil)) m := newAutoMigrator(t, db, migrate.WithModel( - (*Renamed)(nil), (*Model2)(nil), + (*Renamed)(nil), )) // Act @@ -576,273 +577,356 @@ func testRenameColumnRenamesFK(t *testing.T, db *bun.DB) { require.Equal(t, "tennants_my_neighbour_fkey", fkName) } -// TODO: rewrite these tests into AutoMigrator tests, Diff should be moved to migrate/internal package -func TestDiff(t *testing.T) { - type Journal struct { - ISBN string `bun:"isbn,pk"` - Title string `bun:"title,notnull"` - Pages int `bun:"page_count,notnull,default:0"` - } - - type Reader struct { - Username string `bun:",pk,default:gen_random_uuid()"` - } - - type ExternalUsers struct { - bun.BaseModel `bun:"external.users"` - Name string `bun:",pk"` - } - - // ------------------------------------------------------------------------ - type ThingNoOwner struct { - bun.BaseModel `bun:"things"` - ID int64 `bun:"thing_id,pk"` - OwnerID int64 `bun:",notnull"` - } - - type Owner struct { - ID int64 `bun:",pk"` - } - - type Thing struct { - bun.BaseModel `bun:"things"` - ID int64 `bun:"thing_id,pk"` - OwnerID int64 `bun:",notnull"` - - Owner *Owner `bun:"rel:belongs-to,join:owner_id=id"` - } - - testEachDialect(t, func(t *testing.T, dialectName string, dialect schema.Dialect) { - defaultSchema := dialect.DefaultSchema() - - for _, tt := range []struct { - name string - states func(testing.TB, context.Context, schema.Dialect) (stateDb sqlschema.State, stateModel sqlschema.State) - want []migrate.Operation - }{ - { - name: "1 table renamed, 1 created, 2 dropped", - states: func(tb testing.TB, ctx context.Context, d schema.Dialect) (stateDb sqlschema.State, stateModel sqlschema.State) { - // Database state ------------- - type Subscription struct { - bun.BaseModel `bun:"table:billing.subscriptions"` - } - type Review struct{} - - type Author struct { - Name string `bun:"name"` - } - - // Model state ------------- - type JournalRenamed struct { - bun.BaseModel `bun:"table:journals_renamed"` - - ISBN string `bun:"isbn,pk"` - Title string `bun:"title,notnull"` - Pages int `bun:"page_count,notnull,default:0"` - } - - return getState(tb, ctx, d, - (*Author)(nil), - (*Journal)(nil), - (*Review)(nil), - (*Subscription)(nil), - ), getState(tb, ctx, d, - (*Author)(nil), - (*JournalRenamed)(nil), - (*Reader)(nil), - ) - }, - want: []migrate.Operation{ - &migrate.RenameTable{ - Schema: defaultSchema, - From: "journals", - To: "journals_renamed", - }, - &migrate.CreateTable{ - Model: &Reader{}, // (*Reader)(nil) would be more idiomatic, but schema.Tables - }, - &migrate.DropTable{ - Schema: "billing", - Name: "billing.subscriptions", // TODO: fix once schema is used correctly - }, - &migrate.DropTable{ - Schema: defaultSchema, - Name: "reviews", - }, - }, - }, - { - name: "renaming does not work across schemas", - states: func(tb testing.TB, ctx context.Context, d schema.Dialect) (stateDb sqlschema.State, stateModel sqlschema.State) { - // Users have the same columns as the "added" ExternalUsers. - // However, we should not recognize it as a RENAME, because only models in the same schema can be renamed. - // Instead, this is a DROP + CREATE case. - type Users struct { - bun.BaseModel `bun:"external_users"` - Name string `bun:",pk"` - } - - return getState(tb, ctx, d, - (*Users)(nil), - ), getState(t, ctx, d, - (*ExternalUsers)(nil), - ) +func testChangeColumnType(t *testing.T, db *bun.DB) { + type TableBefore struct { + bun.BaseModel `bun:"table:table"` + + // NewPK int64 `bun:"new_pk,notnull,unique"` + PK int32 `bun:"old_pk,pk,identity"` + DefaultExpr string `bun:"default_expr,default:gen_random_uuid()"` + Timestamp time.Time `bun:"ts"` + StillNullable string `bun:"not_null"` + TypeOverride string `bun:"type:char(100)"` + Logical bool `bun:"default:false"` + // ManyValues []string `bun:",array"` + } + + type TableAfter struct { + bun.BaseModel `bun:"table:table"` + + // NewPK int64 `bun:",pk"` + PK int64 `bun:"old_pk,identity"` // ~~no longer PK (not identity)~~ (wip) + DefaultExpr string `bun:"default_expr,type:uuid,default:uuid_nil()"` // different default + type UUID + Timestamp time.Time `bun:"ts,default:current_timestamp"` // has default value now + NotNullable string `bun:"not_null,notnull"` // added NOT NULL + TypeOverride string `bun:"type:char(200)"` // new length + Logical uint8 `bun:"default:1"` // change type + different default + // ManyValues []string `bun:",array"` // did not change + } + + wantTables := []sqlschema.Table{ + { + Schema: db.Dialect().DefaultSchema(), + Name: "table", + Columns: map[string]sqlschema.Column{ + // "new_pk": { + // IsPK: true, + // SQLType: "bigint", + // }, + "old_pk": { + SQLType: "bigint", + IsPK: true, }, - want: []migrate.Operation{ - &migrate.DropTable{ - Schema: defaultSchema, - Name: "external_users", - }, - &migrate.CreateTable{ - Model: &ExternalUsers{}, - }, + "default_expr": { + SQLType: "uuid", + IsNullable: true, + DefaultValue: "uuid_nil()", }, - }, - { - name: "detect new FKs on existing columns", - states: func(t testing.TB, ctx context.Context, d schema.Dialect) (stateDb sqlschema.State, stateModel sqlschema.State) { - // database state - type LonelyUser struct { - bun.BaseModel `bun:"table:users"` - Username string `bun:",pk"` - DreamPetKind string `bun:"pet_kind,notnull"` - DreamPetName string `bun:"pet_name,notnull"` - ImaginaryFriend string `bun:"friend"` - } - - type Pet struct { - Nickname string `bun:",pk"` - Kind string `bun:",pk"` - } - - // model state - type HappyUser struct { - bun.BaseModel `bun:"table:users"` - Username string `bun:",pk"` - PetKind string `bun:"pet_kind,notnull"` - PetName string `bun:"pet_name,notnull"` - Friend string `bun:"friend"` - - Pet *Pet `bun:"rel:has-one,join:pet_kind=kind,join:pet_name=nickname"` - BestFriend *HappyUser `bun:"rel:has-one,join:friend=username"` - } - - return getState(t, ctx, d, - (*LonelyUser)(nil), - (*Pet)(nil), - ), getState(t, ctx, d, - (*HappyUser)(nil), - (*Pet)(nil), - ) - }, - want: []migrate.Operation{ - &migrate.AddFK{ - FK: sqlschema.FK{ - From: sqlschema.C(defaultSchema, "users", "pet_kind", "pet_name"), - To: sqlschema.C(defaultSchema, "pets", "kind", "nickname"), - }, - ConstraintName: "users_pet_kind_pet_name_fkey", - }, - &migrate.AddFK{ - FK: sqlschema.FK{ - From: sqlschema.C(defaultSchema, "users", "friend"), - To: sqlschema.C(defaultSchema, "users", "username"), - }, - ConstraintName: "users_friend_fkey", - }, - }, - }, - { - name: "create FKs for new tables", // TODO: update test case to detect an added column too - states: func(t testing.TB, ctx context.Context, d schema.Dialect) (stateDb sqlschema.State, stateModel sqlschema.State) { - return getState(t, ctx, d, - (*ThingNoOwner)(nil), - ), getState(t, ctx, d, - (*Owner)(nil), - (*Thing)(nil), - ) + "ts": { + SQLType: sqltype.Timestamp, + DefaultValue: "current_timestamp", + IsNullable: true, }, - want: []migrate.Operation{ - &migrate.CreateTable{ - Model: &Owner{}, - }, - &migrate.AddFK{ - FK: sqlschema.FK{ - From: sqlschema.C(defaultSchema, "things", "owner_id"), - To: sqlschema.C(defaultSchema, "owners", "id"), - }, - ConstraintName: "things_owner_id_fkey", - }, + "not_null": { + SQLType: "varchar", }, - }, - { - name: "drop FKs for dropped tables", // TODO: update test case to detect dropped columns too - states: func(t testing.TB, ctx context.Context, d schema.Dialect) (sqlschema.State, sqlschema.State) { - stateDb := getState(t, ctx, d, (*Owner)(nil), (*Thing)(nil)) - stateModel := getState(t, ctx, d, (*ThingNoOwner)(nil)) - - // Normally a database state will have the names of the constraints filled in, but we need to mimic that for the test. - stateDb.FKs[sqlschema.FK{ - From: sqlschema.C(d.DefaultSchema(), "things", "owner_id"), - To: sqlschema.C(d.DefaultSchema(), "owners", "id"), - }] = "test_fkey" - return stateDb, stateModel + "type_override": { + SQLType: "char(200)", + IsNullable: true, }, - want: []migrate.Operation{ - &migrate.DropTable{ - Schema: defaultSchema, - Name: "owners", - }, - &migrate.DropFK{ - FK: sqlschema.FK{ - From: sqlschema.C(defaultSchema, "things", "owner_id"), - To: sqlschema.C(defaultSchema, "owners", "id"), - }, - ConstraintName: "test_fkey", - }, + "logical": { + SQLType: "smallint", + DefaultValue: "1", + IsNullable: true, }, + // "many_values": { + // SQLType: "array", + // }, }, - } { - t.Run(tt.name, func(t *testing.T) { - ctx := context.Background() - stateDb, stateModel := tt.states(t, ctx, dialect) - - got := migrate.Diff(stateDb, stateModel).Operations() - checkEqualChangeset(t, got, tt.want) - }) - } - }) -} - -func checkEqualChangeset(tb testing.TB, got, want []migrate.Operation) { - tb.Helper() + }, + } - // Sort alphabetically to ensure we don't fail because of the wrong order - sort.Slice(got, func(i, j int) bool { - return got[i].String() < got[j].String() - }) - sort.Slice(want, func(i, j int) bool { - return want[i].String() < want[j].String() - }) + ctx := context.Background() + inspect := inspectDbOrSkip(t, db) + mustResetModel(t, ctx, db, (*TableBefore)(nil)) + m := newAutoMigrator(t, db, migrate.WithModel((*TableAfter)(nil))) - var cgot, cwant migrate.Changeset - cgot.Add(got...) - cwant.Add(want...) + // Act + err := m.Run(ctx) + require.NoError(t, err) - require.Equal(tb, cwant.String(), cgot.String()) + // Assert + state := inspect(ctx) + require.Equal(t, wantTables, state.Tables) } -func getState(tb testing.TB, ctx context.Context, dialect schema.Dialect, models ...interface{}) sqlschema.State { - tb.Helper() - - tables := schema.NewTables(dialect) - tables.Register(models...) - - inspector := sqlschema.NewSchemaInspector(tables) - state, err := inspector.Inspect(ctx) - if err != nil { - tb.Skip("get state: %w", err) - } - return state -} +// // TODO: rewrite these tests into AutoMigrator tests, Diff should be moved to migrate/internal package +// func TestDiff(t *testing.T) { +// type Journal struct { +// ISBN string `bun:"isbn,pk"` +// Title string `bun:"title,notnull"` +// Pages int `bun:"page_count,notnull,default:0"` +// } + +// type Reader struct { +// Username string `bun:",pk,default:gen_random_uuid()"` +// } + +// type ExternalUsers struct { +// bun.BaseModel `bun:"external.users"` +// Name string `bun:",pk"` +// } + +// // ------------------------------------------------------------------------ +// type ThingNoOwner struct { +// bun.BaseModel `bun:"things"` +// ID int64 `bun:"thing_id,pk"` +// OwnerID int64 `bun:",notnull"` +// } + +// type Owner struct { +// ID int64 `bun:",pk"` +// } + +// type Thing struct { +// bun.BaseModel `bun:"things"` +// ID int64 `bun:"thing_id,pk"` +// OwnerID int64 `bun:",notnull"` + +// Owner *Owner `bun:"rel:belongs-to,join:owner_id=id"` +// } + +// testEachDialect(t, func(t *testing.T, dialectName string, dialect schema.Dialect) { +// defaultSchema := dialect.DefaultSchema() + +// for _, tt := range []struct { +// name string +// states func(testing.TB, context.Context, schema.Dialect) (stateDb sqlschema.State, stateModel sqlschema.State) +// want []migrate.Operation +// }{ +// { +// name: "1 table renamed, 1 created, 2 dropped", +// states: func(tb testing.TB, ctx context.Context, d schema.Dialect) (stateDb sqlschema.State, stateModel sqlschema.State) { +// // Database state ------------- +// type Subscription struct { +// bun.BaseModel `bun:"table:billing.subscriptions"` +// } +// type Review struct{} + +// type Author struct { +// Name string `bun:"name"` +// } + +// // Model state ------------- +// type JournalRenamed struct { +// bun.BaseModel `bun:"table:journals_renamed"` + +// ISBN string `bun:"isbn,pk"` +// Title string `bun:"title,notnull"` +// Pages int `bun:"page_count,notnull,default:0"` +// } + +// return getState(tb, ctx, d, +// (*Author)(nil), +// (*Journal)(nil), +// (*Review)(nil), +// (*Subscription)(nil), +// ), getState(tb, ctx, d, +// (*Author)(nil), +// (*JournalRenamed)(nil), +// (*Reader)(nil), +// ) +// }, +// want: []migrate.Operation{ +// &migrate.RenameTable{ +// Schema: defaultSchema, +// From: "journals", +// To: "journals_renamed", +// }, +// &migrate.CreateTable{ +// Model: &Reader{}, // (*Reader)(nil) would be more idiomatic, but schema.Tables +// }, +// &migrate.DropTable{ +// Schema: "billing", +// Name: "billing.subscriptions", // TODO: fix once schema is used correctly +// }, +// &migrate.DropTable{ +// Schema: defaultSchema, +// Name: "reviews", +// }, +// }, +// }, +// { +// name: "renaming does not work across schemas", +// states: func(tb testing.TB, ctx context.Context, d schema.Dialect) (stateDb sqlschema.State, stateModel sqlschema.State) { +// // Users have the same columns as the "added" ExternalUsers. +// // However, we should not recognize it as a RENAME, because only models in the same schema can be renamed. +// // Instead, this is a DROP + CREATE case. +// type Users struct { +// bun.BaseModel `bun:"external_users"` +// Name string `bun:",pk"` +// } + +// return getState(tb, ctx, d, +// (*Users)(nil), +// ), getState(t, ctx, d, +// (*ExternalUsers)(nil), +// ) +// }, +// want: []migrate.Operation{ +// &migrate.DropTable{ +// Schema: defaultSchema, +// Name: "external_users", +// }, +// &migrate.CreateTable{ +// Model: &ExternalUsers{}, +// }, +// }, +// }, +// { +// name: "detect new FKs on existing columns", +// states: func(t testing.TB, ctx context.Context, d schema.Dialect) (stateDb sqlschema.State, stateModel sqlschema.State) { +// // database state +// type LonelyUser struct { +// bun.BaseModel `bun:"table:users"` +// Username string `bun:",pk"` +// DreamPetKind string `bun:"pet_kind,notnull"` +// DreamPetName string `bun:"pet_name,notnull"` +// ImaginaryFriend string `bun:"friend"` +// } + +// type Pet struct { +// Nickname string `bun:",pk"` +// Kind string `bun:",pk"` +// } + +// // model state +// type HappyUser struct { +// bun.BaseModel `bun:"table:users"` +// Username string `bun:",pk"` +// PetKind string `bun:"pet_kind,notnull"` +// PetName string `bun:"pet_name,notnull"` +// Friend string `bun:"friend"` + +// Pet *Pet `bun:"rel:has-one,join:pet_kind=kind,join:pet_name=nickname"` +// BestFriend *HappyUser `bun:"rel:has-one,join:friend=username"` +// } + +// return getState(t, ctx, d, +// (*LonelyUser)(nil), +// (*Pet)(nil), +// ), getState(t, ctx, d, +// (*HappyUser)(nil), +// (*Pet)(nil), +// ) +// }, +// want: []migrate.Operation{ +// &migrate.AddFK{ +// FK: sqlschema.FK{ +// From: sqlschema.C(defaultSchema, "users", "pet_kind", "pet_name"), +// To: sqlschema.C(defaultSchema, "pets", "kind", "nickname"), +// }, +// ConstraintName: "users_pet_kind_pet_name_fkey", +// }, +// &migrate.AddFK{ +// FK: sqlschema.FK{ +// From: sqlschema.C(defaultSchema, "users", "friend"), +// To: sqlschema.C(defaultSchema, "users", "username"), +// }, +// ConstraintName: "users_friend_fkey", +// }, +// }, +// }, +// { +// name: "create FKs for new tables", // TODO: update test case to detect an added column too +// states: func(t testing.TB, ctx context.Context, d schema.Dialect) (stateDb sqlschema.State, stateModel sqlschema.State) { +// return getState(t, ctx, d, +// (*ThingNoOwner)(nil), +// ), getState(t, ctx, d, +// (*Owner)(nil), +// (*Thing)(nil), +// ) +// }, +// want: []migrate.Operation{ +// &migrate.CreateTable{ +// Model: &Owner{}, +// }, +// &migrate.AddFK{ +// FK: sqlschema.FK{ +// From: sqlschema.C(defaultSchema, "things", "owner_id"), +// To: sqlschema.C(defaultSchema, "owners", "id"), +// }, +// ConstraintName: "things_owner_id_fkey", +// }, +// }, +// }, +// { +// name: "drop FKs for dropped tables", // TODO: update test case to detect dropped columns too +// states: func(t testing.TB, ctx context.Context, d schema.Dialect) (sqlschema.State, sqlschema.State) { +// stateDb := getState(t, ctx, d, (*Owner)(nil), (*Thing)(nil)) +// stateModel := getState(t, ctx, d, (*ThingNoOwner)(nil)) + +// // Normally a database state will have the names of the constraints filled in, but we need to mimic that for the test. +// stateDb.FKs[sqlschema.FK{ +// From: sqlschema.C(d.DefaultSchema(), "things", "owner_id"), +// To: sqlschema.C(d.DefaultSchema(), "owners", "id"), +// }] = "test_fkey" +// return stateDb, stateModel +// }, +// want: []migrate.Operation{ +// &migrate.DropTable{ +// Schema: defaultSchema, +// Name: "owners", +// }, +// &migrate.DropFK{ +// FK: sqlschema.FK{ +// From: sqlschema.C(defaultSchema, "things", "owner_id"), +// To: sqlschema.C(defaultSchema, "owners", "id"), +// }, +// ConstraintName: "test_fkey", +// }, +// }, +// }, +// } { +// t.Run(tt.name, func(t *testing.T) { +// ctx := context.Background() +// stateDb, stateModel := tt.states(t, ctx, dialect) + +// got := migrate.Diff(stateDb, stateModel).Operations() +// checkEqualChangeset(t, got, tt.want) +// }) +// } +// }) +// } + +// func checkEqualChangeset(tb testing.TB, got, want []migrate.Operation) { +// tb.Helper() + +// // Sort alphabetically to ensure we don't fail because of the wrong order +// sort.Slice(got, func(i, j int) bool { +// return got[i].String() < got[j].String() +// }) +// sort.Slice(want, func(i, j int) bool { +// return want[i].String() < want[j].String() +// }) + +// var cgot, cwant migrate.Changeset +// cgot.Add(got...) +// cwant.Add(want...) + +// require.Equal(tb, cwant.String(), cgot.String()) +// } + +// func getState(tb testing.TB, ctx context.Context, dialect schema.Dialect, models ...interface{}) sqlschema.State { +// tb.Helper() + +// tables := schema.NewTables(dialect) +// tables.Register(models...) + +// inspector := sqlschema.NewSchemaInspector(tables) +// state, err := inspector.Inspect(ctx) +// if err != nil { +// tb.Skip("get state: %w", err) +// } +// return state +// } diff --git a/migrate/alt/operations.go b/migrate/alt/operations.go new file mode 100644 index 000000000..f7f1a8873 --- /dev/null +++ b/migrate/alt/operations.go @@ -0,0 +1,263 @@ +package alt + +import ( + "github.com/uptrace/bun" + "github.com/uptrace/bun/migrate/sqlschema" + "github.com/uptrace/bun/schema" +) + +// Operation encapsulates the request to change a database definition +// and knowns which operation can revert it. +type Operation interface { + GetReverse() Operation +} + +// CreateTable +type CreateTable struct { + Schema string + Name string + Model interface{} +} + +var _ Operation = (*CreateTable)(nil) + +func (op *CreateTable) FQN() schema.FQN { + return schema.FQN{ + Schema: op.Schema, + Table: op.Name, + } +} + +func (op *CreateTable) GetReverse() Operation { + return &DropTable{ + Schema: op.Schema, + Name: op.Name, + } +} + +type DropTable struct { + Schema string + Name string +} + +var _ Operation = (*DropTable)(nil) + +func (op *DropTable) FQN() schema.FQN { + return schema.FQN{ + Schema: op.Schema, + Table: op.Name, + } +} + +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)) +} + +// GetReverse for a DropTable returns a no-op migration. Logically, CreateTable is the reverse, +// but DropTable does not have the table's definition to create one. +// +// TODO: we can fetch table definitions for deleted tables +// from the database engine and execute them as a raw query. +func (op *DropTable) GetReverse() Operation { + return &noop{} +} + +type RenameTable struct { + Schema string + OldName string + NewName string +} + +var _ Operation = (*RenameTable)(nil) +var _ sqlschema.Operation = (*RenameTable)(nil) + +func (op *RenameTable) FQN() schema.FQN { + return schema.FQN{ + Schema: op.Schema, + Table: op.OldName, + } +} + +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, + OldName: op.NewName, + NewName: op.OldName, + } +} + +// RenameColumn. +type RenameColumn struct { + Schema string + Table string + OldName string + NewName string +} + +var _ Operation = (*RenameColumn)(nil) +var _ sqlschema.Operation = (*RenameColumn)(nil) + +func (op *RenameColumn) FQN() schema.FQN { + return schema.FQN{ + Schema: op.Schema, + Table: op.Table, + } +} + +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, + Table: op.Table, + OldName: op.NewName, + NewName: op.OldName, + } +} + +func (op *RenameColumn) DependsOn(another Operation) bool { + rt, ok := another.(*RenameTable) + return ok && rt.Schema == op.Schema && rt.NewName == op.Table +} + +// RenameConstraint. +type RenameConstraint struct { + FK sqlschema.FK + OldName string + NewName string +} + +var _ Operation = (*RenameConstraint)(nil) +var _ sqlschema.Operation = (*RenameConstraint)(nil) + +func (op *RenameConstraint) FQN() schema.FQN { + return schema.FQN{ + Schema: op.FK.From.Schema, + Table: op.FK.From.Table, + } +} + +func (op *RenameConstraint) DependsOn(another Operation) bool { + rt, ok := another.(*RenameTable) + 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, + OldName: op.OldName, + NewName: op.NewName, + } +} + +type AddForeignKey struct { + FK sqlschema.FK + ConstraintName string +} + +var _ Operation = (*AddForeignKey)(nil) +var _ sqlschema.Operation = (*AddForeignKey)(nil) + +func (op *AddForeignKey) FQN() schema.FQN { + return schema.FQN{ + Schema: op.FK.From.Schema, + Table: op.FK.From.Table, + } +} + +func (op *AddForeignKey) DependsOn(another Operation) bool { + switch another := another.(type) { + case *RenameTable: + return another.Schema == op.FK.From.Schema && another.NewName == op.FK.From.Table + case *CreateTable: + return (another.Schema == op.FK.To.Schema && another.Name == op.FK.To.Table) || // either it's the referencing one + (another.Schema == op.FK.From.Schema && another.Name == op.FK.From.Table) // or the one being referenced + } + 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, + ConstraintName: op.ConstraintName, + } +} + +// DropConstraint. +type DropConstraint struct { + FK sqlschema.FK + ConstraintName string +} + +var _ Operation = (*DropConstraint)(nil) +var _ sqlschema.Operation = (*DropConstraint)(nil) + +func (op *DropConstraint) FQN() schema.FQN { + return schema.FQN{ + Schema: op.FK.From.Schema, + Table: op.FK.From.Table, + } +} + +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, + ConstraintName: op.ConstraintName, + } +} + +type ChangeColumnType struct { + Schema string + Table string + Column string + From sqlschema.Column + To sqlschema.Column +} + +var _ Operation = (*ChangeColumnType)(nil) + +func (op *ChangeColumnType) GetReverse() Operation { + return &ChangeColumnType{ + Schema: op.Schema, + Table: op.Table, + Column: op.Column, + From: op.To, + To: op.From, + } +} + +// noop is a migration that doesn't change the schema. +type noop struct{} + +var _ Operation = (*noop)(nil) + +func (*noop) GetReverse() Operation { return &noop{} } diff --git a/migrate/auto.go b/migrate/auto.go index 5750cab00..edb8f9f77 100644 --- a/migrate/auto.go +++ b/migrate/auto.go @@ -3,7 +3,6 @@ package migrate import ( "context" "fmt" - "strings" "github.com/uptrace/bun" "github.com/uptrace/bun/migrate/sqlschema" @@ -36,7 +35,7 @@ func WithExcludeTable(tables ...string) AutoMigratorOption { // which is the default strategy. Perhaps it would make sense to allow disabling this and switching to separate (CreateTable + AddFK) func WithFKNameFunc(f func(sqlschema.FK) string) AutoMigratorOption { return func(m *AutoMigrator) { - m.diffOpts = append(m.diffOpts, FKNameFunc(f)) + m.diffOpts = append(m.diffOpts, fKNameFunc(f)) } } @@ -45,7 +44,7 @@ func WithFKNameFunc(f func(sqlschema.FK) string) AutoMigratorOption { // and in those cases simply renaming the FK makes a lot more sense. func WithRenameFK(enabled bool) AutoMigratorOption { return func(m *AutoMigrator) { - m.diffOpts = append(m.diffOpts, DetectRenamedFKs(enabled)) + m.diffOpts = append(m.diffOpts, detectRenamedFKs(enabled)) } } @@ -94,8 +93,8 @@ type AutoMigrator struct { // excludeTables are excluded from database inspection. excludeTables []string - // diffOpts are passed to Diff. - diffOpts []DiffOption + // diffOpts are passed to detector constructor. + diffOpts []diffOption // migratorOpts are passed to Migrator constructor. migratorOpts []MigratorOption @@ -132,27 +131,32 @@ func NewAutoMigrator(db *bun.DB, opts ...AutoMigratorOption) (*AutoMigrator, err return am, nil } -func (am *AutoMigrator) diff(ctx context.Context) (Changeset, error) { - var changes Changeset +func (am *AutoMigrator) plan(ctx context.Context) (*changeset, error) { var err error got, err := am.dbInspector.Inspect(ctx) if err != nil { - return changes, err + return nil, err } want, err := am.modelInspector.Inspect(ctx) if err != nil { - return changes, err + return nil, err + } + + detector := newDetector(got, want, am.diffOpts...) + changes := detector.Diff() + if err := changes.ResolveDependencies(); err != nil { + return nil, fmt.Errorf("plan migrations: %w", err) } - return Diff(got, want, am.diffOpts...), nil + return changes, nil } // Migrate writes required changes to a new migration file and runs the migration. // This will create and entry in the migrations table, making it possible to revert // the changes with Migrator.Rollback(). func (am *AutoMigrator) Migrate(ctx context.Context, opts ...MigrationOption) error { - changeset, err := am.diff(ctx) + changes, err := am.plan(ctx) if err != nil { return fmt.Errorf("auto migrate: %w", err) } @@ -161,8 +165,8 @@ func (am *AutoMigrator) Migrate(ctx context.Context, opts ...MigrationOption) er name, _ := genMigrationName("auto") migrations.Add(Migration{ Name: name, - Up: changeset.Up(am.dbMigrator), - Down: changeset.Down(am.dbMigrator), + Up: changes.Up(am.dbMigrator), + Down: changes.Down(am.dbMigrator), Comment: "Changes detected by bun.migrate.AutoMigrator", }) @@ -179,570 +183,13 @@ func (am *AutoMigrator) Migrate(ctx context.Context, opts ...MigrationOption) er // Run runs required migrations in-place and without creating a database entry. func (am *AutoMigrator) Run(ctx context.Context) error { - changeset, err := am.diff(ctx) + changes, err := am.plan(ctx) if err != nil { - return fmt.Errorf("run auto migrate: %w", err) + return fmt.Errorf("auto migrate: %w", err) } - up := changeset.Up(am.dbMigrator) + up := changes.Up(am.dbMigrator) if err := up(ctx, am.db); err != nil { - return fmt.Errorf("run auto migrate: %w", err) + return fmt.Errorf("auto migrate: %w", err) } return nil -} - -// INTERNAL ------------------------------------------------------------------- -// TODO: move to migrate/internal - -type DiffOption func(*detectorConfig) - -func FKNameFunc(f func(sqlschema.FK) string) DiffOption { - return func(cfg *detectorConfig) { - cfg.FKNameFunc = f - } -} - -func DetectRenamedFKs(enabled bool) DiffOption { - return func(cfg *detectorConfig) { - cfg.DetectRenamedFKs = enabled - } -} - -func Diff(got, want sqlschema.State, opts ...DiffOption) Changeset { - detector := newDetector(got, want, opts...) - return detector.DetectChanges() -} - -// detectorConfig controls how differences in the model states are resolved. -type detectorConfig struct { - FKNameFunc func(sqlschema.FK) string - DetectRenamedFKs bool -} - -type detector struct { - // current state represents the existing database schema. - current sqlschema.State - - // target state represents the database schema defined in bun models. - target sqlschema.State - - changes Changeset - refMap sqlschema.RefMap - - // fkNameFunc builds the name for created/renamed FK contraints. - fkNameFunc func(sqlschema.FK) string - - // detectRenemedFKS controls how FKs are treated when their references (table/column) are renamed. - detectRenamedFKs bool -} - -func newDetector(got, want sqlschema.State, opts ...DiffOption) *detector { - cfg := &detectorConfig{ - FKNameFunc: defaultFKName, - DetectRenamedFKs: false, - } - for _, opt := range opts { - opt(cfg) - } - - var existingFKs []sqlschema.FK - for fk := range got.FKs { - existingFKs = append(existingFKs, fk) - } - - return &detector{ - current: got, - target: want, - refMap: sqlschema.NewRefMap(existingFKs...), - fkNameFunc: cfg.FKNameFunc, - detectRenamedFKs: cfg.DetectRenamedFKs, - } -} - -func (d *detector) DetectChanges() Changeset { - // Discover CREATE/RENAME/DROP TABLE - 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. - created := newTableSet() - renamed := newTableSet() - - addedTables := targetTables.Sub(currentTables) -AddedLoop: - for _, added := range addedTables.Values() { - removedTables := currentTables.Sub(targetTables) - for _, removed := range removedTables.Values() { - if d.canRename(removed, added) { - d.changes.Add(&RenameTable{ - Schema: removed.Schema, - From: removed.Name, - To: added.Name, - }) - - d.detectRenamedColumns(removed, added) - - // Update referenced table in all related FKs - if d.detectRenamedFKs { - d.refMap.UpdateT(removed.T(), added.T()) - } - - renamed.Add(added) - - // Do not check this model further, we know it was renamed. - currentTables.Remove(removed.Name) - continue AddedLoop - } - } - // If a new table did not appear because of the rename operation, then it must've been created. - d.changes.Add(&CreateTable{ - Schema: added.Schema, - Name: added.Name, - Model: added.Model, - }) - created.Add(added) - } - - // 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(&DropTable{ - Schema: t.Schema, - Name: t.Name, - }) - } - - // 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 - var current sqlschema.Table - for _, cur := range curEx.Values() { - if cur.Name == target.Name { - current = cur - break - } - } - d.detectRenamedColumns(current, target) - } - - // Compare and update FKs ---------------- - currentFKs := make(map[sqlschema.FK]string) - for k, v := range d.current.FKs { - currentFKs[k] = v - } - - if d.detectRenamedFKs { - // Add RenameFK migrations for updated FKs. - for old, renamed := range d.refMap.Updated() { - newName := d.fkNameFunc(renamed) - d.changes.Add(&RenameFK{ - FK: renamed, // TODO: make sure this is applied after the table/columns are renamed - From: d.current.FKs[old], - To: d.fkNameFunc(renamed), - }) - - // Here we can add this fk to "current.FKs" to prevent it from firing in the next 2 for-loops. - currentFKs[renamed] = newName - delete(currentFKs, old) - } - } - - // Add AddFK migrations for newly added FKs. - for fk := range d.target.FKs { - if _, ok := currentFKs[fk]; !ok { - d.changes.Add(&AddFK{ - FK: fk, - ConstraintName: d.fkNameFunc(fk), - }) - } - } - - // Add DropFK migrations for removed FKs. - for fk, fkName := range currentFKs { - if _, ok := d.target.FKs[fk]; !ok { - d.changes.Add(&DropFK{ - FK: fk, - ConstraintName: fkName, - }) - } - } - - return d.changes -} - -// canRename checks if t1 can be renamed to t2. -func (d detector) canRename(t1, t2 sqlschema.Table) bool { - return t1.Schema == t2.Schema && sqlschema.EqualSignatures(t1, t2) -} - -func (d *detector) detectRenamedColumns(removed, added sqlschema.Table) { - for aName, aCol := range added.Columns { - // This column exists in the database, so it wasn't renamed - if _, ok := removed.Columns[aName]; ok { - continue - } - for rName, rCol := range removed.Columns { - if aCol != rCol { - continue - } - d.changes.Add(&RenameColumn{ - Schema: added.Schema, - Table: added.Name, - From: rName, - To: aName, - }) - delete(removed.Columns, rName) // no need to check this column again - d.refMap.UpdateC(sqlschema.C(added.Schema, added.Name, rName), aName) - } - } -} - -// Changeset is a set of changes that alter database state. -type Changeset struct { - operations []Operation -} - -var _ Operation = (*Changeset)(nil) - -func (c Changeset) String() string { - var ops []string - for _, op := range c.operations { - ops = append(ops, op.String()) - } - if len(ops) == 0 { - return "" - } - return strings.Join(ops, "\n") -} - -func (c Changeset) Operations() []Operation { - return c.operations -} - -// Add new operations to the changeset. -func (c *Changeset) Add(op ...Operation) { - c.operations = append(c.operations, op...) -} - -// Func chains all underlying operations in a single MigrationFunc. -func (c *Changeset) Func(m sqlschema.Migrator) MigrationFunc { - return func(ctx context.Context, db *bun.DB) error { - for _, op := range c.operations { - fn := op.Func(m) - if err := fn(ctx, db); err != nil { - return err - } - } - return nil - } -} - -func (c *Changeset) GetReverse() Operation { - var reverse Changeset - for _, op := range c.operations { - reverse.Add(op.GetReverse()) - } - return &reverse -} - -// Up is syntactic sugar. -func (c *Changeset) Up(m sqlschema.Migrator) MigrationFunc { - return c.Func(m) -} - -// Down is syntactic sugar. -func (c *Changeset) Down(m sqlschema.Migrator) MigrationFunc { - return c.GetReverse().Func(m) -} - -// Operation is an abstraction a level above a MigrationFunc. -// Apart from storing the function to execute the change, -// it knows how to *write* the corresponding code, and what the reverse operation is. -type Operation interface { - fmt.Stringer - - Func(sqlschema.Migrator) MigrationFunc - // GetReverse returns an operation that can revert the current one. - GetReverse() Operation -} - -// noop is a migration that doesn't change the schema. -type noop struct{} - -var _ Operation = (*noop)(nil) - -func (*noop) String() string { return "noop" } -func (*noop) Func(m sqlschema.Migrator) MigrationFunc { - return func(ctx context.Context, db *bun.DB) error { return nil } -} -func (*noop) GetReverse() Operation { return &noop{} } - -type RenameTable struct { - Schema string - From string - To string -} - -var _ Operation = (*RenameTable)(nil) - -func (op RenameTable) String() string { - return fmt.Sprintf( - "Rename table %q.%q to %q.%q", - op.Schema, trimSchema(op.From), op.Schema, trimSchema(op.To), - ) -} - -func (op *RenameTable) Func(m sqlschema.Migrator) MigrationFunc { - return func(ctx context.Context, db *bun.DB) error { - return m.RenameTable(ctx, op.From, op.To) - } -} - -func (op *RenameTable) GetReverse() Operation { - return &RenameTable{ - Schema: op.Schema, - From: op.To, - To: op.From, - } -} - -type CreateTable struct { - Schema string - Name string - Model interface{} -} - -var _ Operation = (*CreateTable)(nil) - -func (op CreateTable) String() string { - return fmt.Sprintf("CreateTable %T", op.Model) -} - -func (op *CreateTable) Func(m sqlschema.Migrator) MigrationFunc { - return func(ctx context.Context, db *bun.DB) error { - return m.CreateTable(ctx, op.Model) - } -} - -func (op *CreateTable) GetReverse() Operation { - return &DropTable{ - Schema: op.Schema, - Name: op.Name, - } -} - -type DropTable struct { - Schema string - Name string -} - -var _ Operation = (*DropTable)(nil) - -func (op DropTable) String() string { - return fmt.Sprintf("DropTable %q.%q", op.Schema, trimSchema(op.Name)) -} - -func (op *DropTable) Func(m sqlschema.Migrator) MigrationFunc { - return func(ctx context.Context, db *bun.DB) error { - return m.DropTable(ctx, op.Schema, op.Name) - } -} - -// GetReverse for a DropTable returns a no-op migration. Logically, CreateTable is the reverse, -// but DropTable does not have the table's definition to create one. -// -// TODO: we can fetch table definitions for deleted tables -// from the database engine and execute them as a raw query. -func (op *DropTable) GetReverse() Operation { - return &noop{} -} - -// trimSchema drops schema name from the table name. -// This is a workaroud until schema.Table.Schema is fully integrated with other bun packages. -func trimSchema(name string) string { - if strings.Contains(name, ".") { - return strings.Split(name, ".")[1] - } - return name -} - -// defaultFKName returns a name for the FK constraint in the format {tablename}_{columnname(s)}_fkey, following the Postgres convention. -func defaultFKName(fk sqlschema.FK) string { - columnnames := strings.Join(fk.From.Column.Split(), "_") - return fmt.Sprintf("%s_%s_fkey", fk.From.Table, columnnames) -} - -type AddFK struct { - FK sqlschema.FK - ConstraintName string -} - -var _ Operation = (*AddFK)(nil) - -func (op AddFK) String() string { - source, target := op.FK.From, op.FK.To - return fmt.Sprintf("AddForeignKey %q %s.%s(%s) references %s.%s(%s)", op.ConstraintName, - source.Schema, source.Table, strings.Join(source.Column.Split(), ","), - target.Schema, target.Table, strings.Join(target.Column.Split(), ","), - ) -} - -func (op *AddFK) Func(m sqlschema.Migrator) MigrationFunc { - return func(ctx context.Context, db *bun.DB) error { - return m.AddContraint(ctx, op.FK, op.ConstraintName) - } -} - -func (op *AddFK) GetReverse() Operation { - return &DropFK{ - FK: op.FK, - ConstraintName: op.ConstraintName, - } -} - -type DropFK struct { - FK sqlschema.FK - ConstraintName string -} - -var _ Operation = (*DropFK)(nil) - -func (op *DropFK) String() string { - source := op.FK.From.T() - return fmt.Sprintf("DropFK %q on table %q.%q", op.ConstraintName, source.Schema, source.Table) -} - -func (op *DropFK) Func(m sqlschema.Migrator) MigrationFunc { - return func(ctx context.Context, db *bun.DB) error { - source := op.FK.From.T() - return m.DropContraint(ctx, source.Schema, source.Table, op.ConstraintName) - } -} - -func (op *DropFK) GetReverse() Operation { - return &AddFK{ - FK: op.FK, - ConstraintName: op.ConstraintName, - } -} - -// RenameFK -type RenameFK struct { - FK sqlschema.FK - From string - To string -} - -var _ Operation = (*RenameFK)(nil) - -func (op *RenameFK) String() string { - return "RenameFK" -} - -func (op *RenameFK) Func(m sqlschema.Migrator) MigrationFunc { - return func(ctx context.Context, db *bun.DB) error { - table := op.FK.From - return m.RenameConstraint(ctx, table.Schema, table.Table, op.From, op.To) - } -} - -func (op *RenameFK) GetReverse() Operation { - return &RenameFK{ - FK: op.FK, - From: op.From, - To: op.To, - } -} - -// RenameColumn -type RenameColumn struct { - Schema string - Table string - From string - To string -} - -var _ Operation = (*RenameColumn)(nil) - -func (op RenameColumn) String() string { - return "" -} - -func (op *RenameColumn) Func(m sqlschema.Migrator) MigrationFunc { - return func(ctx context.Context, db *bun.DB) error { - return m.RenameColumn(ctx, op.Schema, op.Table, op.From, op.To) - } -} - -func (op *RenameColumn) GetReverse() Operation { - return &RenameColumn{ - Schema: op.Schema, - Table: op.Table, - From: op.To, - To: op.From, - } -} - -// sqlschema utils ------------------------------------------------------------ - -// tableSet stores unique table definitions. -type tableSet struct { - underlying map[string]sqlschema.Table -} - -func newTableSet(initial ...sqlschema.Table) tableSet { - set := tableSet{ - underlying: make(map[string]sqlschema.Table), - } - for _, t := range initial { - set.Add(t) - } - return set -} - -func (set tableSet) Add(t sqlschema.Table) { - set.underlying[t.Name] = t -} - -func (set tableSet) Remove(s string) { - delete(set.underlying, s) -} - -func (set tableSet) Values() (tables []sqlschema.Table) { - for _, t := range set.underlying { - tables = append(tables, t) - } - return -} - -func (set tableSet) Sub(other tableSet) tableSet { - res := set.clone() - for v := range other.underlying { - if _, ok := set.underlying[v]; ok { - res.Remove(v) - } - } - return res -} - -func (set tableSet) clone() tableSet { - res := newTableSet() - for _, t := range set.underlying { - res.Add(t) - } - return res -} - -func (set tableSet) String() string { - var s strings.Builder - for k := range set.underlying { - if s.Len() > 0 { - s.WriteString(", ") - } - s.WriteString(k) - } - return s.String() -} +} \ No newline at end of file diff --git a/migrate/diff.go b/migrate/diff.go new file mode 100644 index 000000000..4c875975c --- /dev/null +++ b/migrate/diff.go @@ -0,0 +1,390 @@ +package migrate + +import ( + "context" + "errors" + "fmt" + "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 +} + +// Add new operations to the changeset. +func (c *changeset) Add(op ...alt.Operation) { + c.operations = append(c.operations, op...) +} + +// Func creates a MigrationFunc that applies all operations all the changeset. +func (c *changeset) Func(m sqlschema.Migrator) MigrationFunc { + return func(ctx context.Context, db *bun.DB) error { + var operations []sqlschema.Operation + for _, op := range c.operations { + operations = append(operations, op.(sqlschema.Operation)) + } + return m.Apply(ctx, operations...) + } +} + +// Up is syntactic sugar. +func (c *changeset) Up(m sqlschema.Migrator) MigrationFunc { + return c.Func(m) +} + +// Down is syntactic sugar. +func (c *changeset) Down(m sqlschema.Migrator) MigrationFunc { + var reverse changeset + for i := len(c.operations) - 1; i >= 0; i-- { + reverse.Add(c.operations[i].GetReverse()) + } + return reverse.Func(m) +} + +func (c *changeset) ResolveDependencies() error { + if len(c.operations) <= 1 { + return nil + } + + const ( + unvisited = iota + current + visited + ) + + var resolved []alt.Operation + var visit func(op alt.Operation) error + + var nextOp alt.Operation + var next func() bool + + status := make(map[alt.Operation]int, len(c.operations)) + for _, op := range c.operations { + status[op] = unvisited + } + + next = func() bool { + for op, s := range status { + if s == unvisited { + nextOp = op + return true + } + } + return false + } + + // 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 { + switch status[op] { + case visited: + return nil + case current: + // TODO: add details (circle) to the error message + return errors.New("detected circular dependency") + } + + status[op] = current + + for _, another := range c.operations { + if dop, hasDeps := another.(interface { + DependsOn(alt.Operation) bool + }); another == op || !hasDeps || !dop.DependsOn(op) { + continue + } + if err := visit(another); err != nil { + return err + } + } + + 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...) + return nil + } + + for next() { + if err := visit(nextOp); err != nil { + return err + } + } + + c.operations = resolved + return nil +} + +type diffOption func(*detectorConfig) + +func fKNameFunc(f func(sqlschema.FK) string) diffOption { + return func(cfg *detectorConfig) { + cfg.FKNameFunc = f + } +} + +func detectRenamedFKs(enabled bool) diffOption { + return func(cfg *detectorConfig) { + cfg.DetectRenamedFKs = enabled + } +} + +// detectorConfig controls how differences in the model states are resolved. +type detectorConfig struct { + FKNameFunc func(sqlschema.FK) string + DetectRenamedFKs bool +} + +type detector struct { + // current state represents the existing database schema. + current sqlschema.State + + // target state represents the database schema defined in bun models. + target sqlschema.State + + changes changeset + refMap sqlschema.RefMap + + // fkNameFunc builds the name for created/renamed FK contraints. + fkNameFunc func(sqlschema.FK) string + + // detectRenemedFKS controls how FKs are treated when their references (table/column) are renamed. + detectRenamedFKs bool +} + +func newDetector(got, want sqlschema.State, opts ...diffOption) *detector { + cfg := &detectorConfig{ + FKNameFunc: defaultFKName, + DetectRenamedFKs: false, + } + for _, opt := range opts { + opt(cfg) + } + + var existingFKs []sqlschema.FK + for fk := range got.FKs { + existingFKs = append(existingFKs, fk) + } + + return &detector{ + current: got, + target: want, + refMap: sqlschema.NewRefMap(existingFKs...), + fkNameFunc: cfg.FKNameFunc, + detectRenamedFKs: cfg.DetectRenamedFKs, + } +} + +func (d *detector) Diff() *changeset { + // Discover CREATE/RENAME/DROP TABLE + 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. + created := newTableSet() + renamed := newTableSet() + + addedTables := targetTables.Sub(currentTables) +AddedLoop: + for _, added := range addedTables.Values() { + removedTables := currentTables.Sub(targetTables) + for _, removed := range removedTables.Values() { + if d.canRename(removed, added) { + d.changes.Add(&alt.RenameTable{ + Schema: removed.Schema, + OldName: removed.Name, + NewName: added.Name, + }) + + d.detectRenamedColumns(removed, added) + + // Update referenced table in all related FKs + if d.detectRenamedFKs { + d.refMap.UpdateT(removed.T(), added.T()) + } + + renamed.Add(added) + + // Do not check this model further, we know it was renamed. + currentTables.Remove(removed.Name) + continue AddedLoop + } + } + // If a new table did not appear because of the rename operation, then it must've been created. + d.changes.Add(&alt.CreateTable{ + Schema: added.Schema, + Name: added.Name, + Model: added.Model, + }) + created.Add(added) + } + + // 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{ + Schema: t.Schema, + Name: t.Name, + }) + } + + // 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 + var current sqlschema.Table + for _, cur := range curEx.Values() { + if cur.Name == target.Name { + current = cur + break + } + } + d.detectRenamedColumns(current, target) + } + + // Compare and update FKs ---------------- + currentFKs := make(map[sqlschema.FK]string) + for k, v := range d.current.FKs { + currentFKs[k] = v + } + + if d.detectRenamedFKs { + // Add RenameFK migrations for updated FKs. + for old, renamed := range d.refMap.Updated() { + newName := d.fkNameFunc(renamed) + d.changes.Add(&alt.RenameConstraint{ + FK: renamed, // TODO: make sure this is applied after the table/columns are renamed + OldName: d.current.FKs[old], + NewName: d.fkNameFunc(renamed), + }) + + // Here we can add this fk to "current.FKs" to prevent it from firing in the next 2 for-loops. + currentFKs[renamed] = newName + delete(currentFKs, old) + } + } + + // Add AddFK migrations for newly added FKs. + for fk := range d.target.FKs { + if _, ok := currentFKs[fk]; !ok { + d.changes.Add(&alt.AddForeignKey{ + FK: fk, + ConstraintName: d.fkNameFunc(fk), + }) + } + } + + // Add DropFK migrations for removed FKs. + for fk, fkName := range currentFKs { + if _, ok := d.target.FKs[fk]; !ok { + d.changes.Add(&alt.DropConstraint{ + FK: fk, + ConstraintName: fkName, + }) + } + } + + return &d.changes +} + +// canRename checks if t1 can be renamed to t2. +func (d detector) canRename(t1, t2 sqlschema.Table) bool { + return t1.Schema == t2.Schema && sqlschema.EqualSignatures(t1, t2) +} + +func (d *detector) detectRenamedColumns(current, added sqlschema.Table) { + for aName, aCol := range added.Columns { + // This column exists in the database, so it wasn't renamed + if _, ok := current.Columns[aName]; ok { + continue + } + for cName, cCol := range current.Columns { + if aCol != cCol { + continue + } + d.changes.Add(&alt.RenameColumn{ + Schema: added.Schema, + Table: added.Name, + OldName: cName, + NewName: aName, + }) + delete(current.Columns, cName) // no need to check this column again + d.refMap.UpdateC(sqlschema.C(added.Schema, added.Name, cName), aName) + break + } + } +} + +// sqlschema utils ------------------------------------------------------------ + +// tableSet stores unique table definitions. +type tableSet struct { + underlying map[string]sqlschema.Table +} + +func newTableSet(initial ...sqlschema.Table) tableSet { + set := tableSet{ + underlying: make(map[string]sqlschema.Table), + } + for _, t := range initial { + set.Add(t) + } + return set +} + +func (set tableSet) Add(t sqlschema.Table) { + set.underlying[t.Name] = t +} + +func (set tableSet) Remove(s string) { + delete(set.underlying, s) +} + +func (set tableSet) Values() (tables []sqlschema.Table) { + for _, t := range set.underlying { + tables = append(tables, t) + } + return +} + +func (set tableSet) Sub(other tableSet) tableSet { + res := set.clone() + for v := range other.underlying { + if _, ok := set.underlying[v]; ok { + res.Remove(v) + } + } + return res +} + +func (set tableSet) clone() tableSet { + res := newTableSet() + for _, t := range set.underlying { + res.Add(t) + } + return res +} + +func (set tableSet) String() string { + var s strings.Builder + for k := range set.underlying { + if s.Len() > 0 { + s.WriteString(", ") + } + s.WriteString(k) + } + return s.String() +} + +// defaultFKName returns a name for the FK constraint in the format {tablename}_{columnname(s)}_fkey, following the Postgres convention. +func defaultFKName(fk sqlschema.FK) string { + columnnames := strings.Join(fk.From.Column.Split(), "_") + return fmt.Sprintf("%s_%s_fkey", fk.From.Table, columnnames) +} diff --git a/migrate/migrator.go b/migrate/migrator.go index b14ad64ca..9f1b5222c 100644 --- a/migrate/migrator.go +++ b/migrate/migrator.go @@ -276,7 +276,7 @@ func (m *Migrator) CreateGoMigration( // CreateTxSQLMigration creates transactional up and down SQL migration files. func (m *Migrator) CreateTxSQLMigrations(ctx context.Context, name string) ([]*MigrationFile, error) { - name, err := m.genMigrationName(name) + name, err := genMigrationName(name) if err != nil { return nil, err } @@ -296,7 +296,7 @@ func (m *Migrator) CreateTxSQLMigrations(ctx context.Context, name string) ([]*M // CreateSQLMigrations creates up and down SQL migration files. func (m *Migrator) CreateSQLMigrations(ctx context.Context, name string) ([]*MigrationFile, error) { - name, err := m.genMigrationName(name) + name, err := genMigrationName(name) if err != nil { return nil, err } diff --git a/migrate/sqlschema/inspector.go b/migrate/sqlschema/inspector.go index 2060fef0c..53fc95a0f 100644 --- a/migrate/sqlschema/inspector.go +++ b/migrate/sqlschema/inspector.go @@ -71,6 +71,13 @@ func (si *SchemaInspector) Inspect(ctx context.Context) (State, error) { }) for _, rel := range t.Relations { + // These relations are nominal and do not need a foreign key to be declared in the current table. + // They will be either expressed as N:1 relations in an m2m mapping table, or will be referenced by the other table if it's a 1:N. + if rel.Type == schema.ManyToManyRelation || + rel.Type == schema.HasManyRelation { + continue + } + var fromCols, toCols []string for _, f := range rel.BaseFields { fromCols = append(fromCols, f.Name) diff --git a/migrate/sqlschema/migrator.go b/migrate/sqlschema/migrator.go index befdb8ad5..3bdeb7e08 100644 --- a/migrate/sqlschema/migrator.go +++ b/migrate/sqlschema/migrator.go @@ -13,7 +13,14 @@ 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 diff --git a/schema/sqlfmt.go b/schema/sqlfmt.go index 7b4a9493f..11eabb13b 100644 --- a/schema/sqlfmt.go +++ b/schema/sqlfmt.go @@ -1,6 +1,7 @@ package schema import ( + "fmt" "strings" "github.com/uptrace/bun/internal" @@ -38,6 +39,24 @@ func (s Name) AppendQuery(fmter Formatter, b []byte) ([]byte, error) { //------------------------------------------------------------------------------ +// FQN represents a fully qualified table name. +type FQN struct { + Schema string + Table string +} + +var _ QueryAppender = (*FQN)(nil) + +func (fqn *FQN) AppendQuery(fmter Formatter, b []byte) ([]byte, error) { + return fmter.AppendQuery(b, "?.?", Ident(fqn.Schema), Ident(fqn.Table)), nil +} + +func (fqn *FQN) String() string { + return fmt.Sprintf("%s.%s", fqn.Schema, fqn.Table) +} + +//------------------------------------------------------------------------------ + // Ident represents a SQL identifier, for example, // a fully qualified column name such as `table_name.col_name`. type Ident string diff --git a/schema/table.go b/schema/table.go index 355f07f6e..d9e1ef01a 100644 --- a/schema/table.go +++ b/schema/table.go @@ -1068,3 +1068,5 @@ func makeIndex(a, b []int) []int { dest = append(dest, b...) return dest } + +