wip: resolve dependencies + refactor

dialect/pgdialect/alter_table.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"log"
 
 	"github.com/uptrace/bun"
 	"github.com/uptrace/bun/migrate/alt"
@@ -25,7 +24,6 @@ type Migrator struct {
 var _ sqlschema.Migrator = (*Migrator)(nil)
 
 func (m *Migrator) execRaw(ctx context.Context, q *bun.RawQuery) error {
-	log.Print("execRaw: ", q.String())
 	if _, err := q.Exec(ctx); err != nil {
 		return err
 	}
@@ -190,7 +188,6 @@ func (q *AlterTableQuery) Chain(op sqlschema.Operation) error {
 	default:
 		return fmt.Errorf("unsupported operation %T", op)
 	}
-	return nil
 }
 
 func (q *AlterTableQuery) isEmpty() bool {

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{

internal/dbtest/migrate_test.go

@@ -477,7 +477,7 @@ func testRenamedColumns(t *testing.T, db *bun.DB) {
 	// Database state
 	type Original struct {
 		bun.BaseModel `bun:"original"`
-		ID int64 `bun:",pk"`
+		ID            int64 `bun:",pk"`
 	}
 
 	type Model1 struct {
@@ -508,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

migrate/alt/operations.go

@@ -21,6 +21,13 @@ type CreateTable struct {
 
 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,
@@ -35,6 +42,19 @@ type DropTable struct {
 
 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.
 //
@@ -108,7 +128,7 @@ func (op *RenameColumn) DependsOn(another Operation) bool {
 	return ok && rt.Schema == op.Schema && rt.NewName == op.Table
 }
 
-// RenameConstraint
+// RenameConstraint.
 type RenameConstraint struct {
 	FK      sqlschema.FK
 	OldName string
@@ -125,6 +145,11 @@ func (op *RenameConstraint) FQN() schema.FQN {
 	}
 }
 
+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
 }
@@ -143,22 +168,67 @@ type AddForeignKey struct {
 }
 
 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 &DropForeignKey{
+	return &DropConstraint{
 		FK:             op.FK,
 		ConstraintName: op.ConstraintName,
 	}
 }
 
-type DropForeignKey struct {
+// DropConstraint.
+type DropConstraint struct {
 	FK             sqlschema.FK
 	ConstraintName string
 }
 
-var _ Operation = (*DropForeignKey)(nil)
+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 *DropForeignKey) GetReverse() Operation {
+func (op *DropConstraint) GetReverse() Operation {
 	return &AddForeignKey{
 		FK:             op.FK,
 		ConstraintName: op.ConstraintName,

migrate/auto.go

@@ -147,7 +147,7 @@ func (am *AutoMigrator) plan(ctx context.Context) (*changeset, error) {
 	detector := newDetector(got, want, am.diffOpts...)
 	changes := detector.Diff()
 	if err := changes.ResolveDependencies(); err != nil {
-		return nil, err
+		return nil, fmt.Errorf("plan migrations: %w", err)
 	}
 	return changes, nil
 }
@@ -185,14 +185,11 @@ func (am *AutoMigrator) Migrate(ctx context.Context, opts ...MigrationOption) er
 func (am *AutoMigrator) Run(ctx context.Context) error {
 	changes, err := am.plan(ctx)
 	if err != nil {
-		return fmt.Errorf("run auto migrate: %w", err)
+		return fmt.Errorf("auto migrate: %w", err)
 	}
 	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
+}

migrate/diff.go

@@ -85,6 +85,7 @@ func (c *changeset) ResolveDependencies() error {
 		case visited:
 			return nil
 		case current:
+			// TODO: add details (circle) to the error message
 			return errors.New("detected circular dependency")
 		}
 
@@ -96,7 +97,9 @@ func (c *changeset) ResolveDependencies() error {
 			}); another == op || !hasDeps || !dop.DependsOn(op) {
 				continue
 			}
-			visit(another)
+			if err := visit(another); err != nil {
+				return err
+			}
 		}
 
 		status[op] = visited
@@ -281,7 +284,7 @@ AddedLoop:
 	// Add DropFK migrations for removed FKs.
 	for fk, fkName := range currentFKs {
 		if _, ok := d.target.FKs[fk]; !ok {
-			d.changes.Add(&alt.DropForeignKey{
+			d.changes.Add(&alt.DropConstraint{
 				FK:             fk,
 				ConstraintName: fkName,
 			})

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)

schema/sqlfmt.go

@@ -1,6 +1,7 @@
 package schema
 
 import (
+	"fmt"
 	"strings"
 
 	"github.com/uptrace/bun/internal"
@@ -50,6 +51,10 @@ 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,