Repository: ariga/atlas
Pull request #416 (2022-01-14 16:39:53)

sql/mysql: support modifying schema charset and collation attributes
We can consider to add schema changes as ModifyCharset and ModifyCollation
in the future to avoid all the type assertion.


Patch:
diff --git a/sql/internal/sqlx/diff.go b/sql/internal/sqlx/diff.go
index 0eec8760..9671401f 100644
--- a/sql/internal/sqlx/diff.go
+++ b/sql/internal/sqlx/diff.go
@@ -102,6 +102,7 @@ func (d *Diff) SchemaDiff(from, to *schema.Schema) ([]schema.Change, error) {
 	// Drop or modify attributes (collations, charset, etc).
 	if change := d.SchemaAttrDiff(from, to); len(change) > 0 {
 		changes = append(changes, &schema.ModifySchema{
+			S:       to,
 			Changes: change,
 		})
 	}
@@ -119,7 +120,7 @@ func (d *Diff) SchemaDiff(from, to *schema.Schema) ([]schema.Change, error) {
 		}
 		if len(change) > 0 {
 			changes = append(changes, &schema.ModifyTable{
-				T:       t1,
+				T:       t2,
 				Changes: change,
 			})
 		}
@@ -459,3 +460,20 @@ func Unquote(s string) (string, error) {
 		return s, nil
 	}
 }
+
+// SingleQuote quotes the given string with single quote.
+func SingleQuote(s string) (string, error) {
+	switch {
+	case IsQuoted(s, '\''):
+		return s, nil
+	case IsQuoted(s, '"'):
+		v, err := strconv.Unquote(s)
+		if err != nil {
+			return "", err
+		}
+		s = v
+		fallthrough
+	default:
+		return "'" + strings.ReplaceAll(s, "'", "''") + "'", nil
+	}
+}
diff --git a/sql/mysql/migrate.go b/sql/mysql/migrate.go
index 3052608c..a5702c6a 100644
--- a/sql/mysql/migrate.go
+++ b/sql/mysql/migrate.go
@@ -61,8 +61,11 @@ type state struct {
 // plan builds the migration plan for applying the
 // given changes on the attached connection.
 func (s *state) plan(changes []schema.Change) error {
-	planned := s.topLevel(changes)
-	planned, err := sqlx.DetachCycles(planned)
+	planned, err := s.topLevel(changes)
+	if err != nil {
+		return err
+	}
+	planned, err = sqlx.DetachCycles(planned)
 	if err != nil {
 		return err
 	}
@@ -86,7 +89,7 @@ func (s *state) plan(changes []schema.Change) error {
 }
 
 // topLevel appends first the changes for creating or dropping schemas (top-level schema elements).
-func (s *state) topLevel(changes []schema.Change) []schema.Change {
+func (s *state) topLevel(changes []schema.Change) ([]schema.Change, error) {
 	planned := make([]schema.Change, 0, len(changes))
 	for _, c := range changes {
 		switch c := c.(type) {
@@ -95,10 +98,12 @@ func (s *state) topLevel(changes []schema.Change) []schema.Change {
 			if sqlx.Has(c.Extra, &schema.IfNotExists{}) {
 				b.P("IF NOT EXISTS")
 			}
-			if a := (schema.Charset{}); sqlx.Has(c.S.Attrs, &a) {
+			// Schema was created with CHARSET and it is not the default database character set.
+			if a := (schema.Charset{}); sqlx.Has(c.S.Attrs, &a) && a.V != "" && a.V != s.charset {
 				b.P("CHARSET", a.V)
 			}
-			if a := (schema.Collation{}); sqlx.Has(c.S.Attrs, &a) {
+			// Schema was created with COLLATE and it is not the default database collation.
+			if a := (schema.Collation{}); sqlx.Has(c.S.Attrs, &a) && a.V != "" && a.V != s.collate {
 				b.P("COLLATE", a.V)
 			}
 			s.append(&migrate.Change{
@@ -117,11 +122,76 @@ func (s *state) topLevel(changes []schema.Change) []schema.Change {
 				Source:  c,
 				Comment: fmt.Sprintf("drop schema named %q", c.S.Name),
 			})
+		case *schema.ModifySchema:
+			if err := s.modifySchema(c); err != nil {
+				return nil, err
+			}
 		default:
 			planned = append(planned, c)
 		}
 	}
-	return planned
+	return planned, nil
+}
+
+// modifySchema builds and appends the migrate.Changes for bringing
+// the schema into its modified state.
+func (s *state) modifySchema(modify *schema.ModifySchema) error {
+	b, r := Build(""), Build("")
+	for _, change := range modify.Changes {
+		switch change := change.(type) {
+		// Add schema attributes to an existing schema only if
+		// it is different from the default server configuration.
+		case *schema.AddAttr:
+			switch a := change.A.(type) {
+			case *schema.Charset:
+				if a.V != "" && a.V != s.charset {
+					b.P("CHARSET", a.V)
+					r.P("CHARSET", s.charset)
+				}
+			case *schema.Collation:
+				if a.V != "" && a.V != s.collate {
+					b.P("COLLATE", a.V)
+					r.P("COLLATE", s.collate)
+				}
+			default:
+				return fmt.Errorf("unexpected schema AddAttr: %T", a)
+			}
+		case *schema.ModifyAttr:
+			switch to := change.To.(type) {
+			case *schema.Charset:
+				from, ok := change.From.(*schema.Charset)
+				if !ok {
+					return fmt.Errorf("mismatch ModifyAttr attributes: %T != %T", change.To, change.From)
+				}
+				b.P("CHARSET", to.V)
+				r.P("CHARSET", from.V)
+			case *schema.Collation:
+				from, ok := change.From.(*schema.Collation)
+				if !ok {
+					return fmt.Errorf("mismatch ModifyAttr attributes: %T != %T", change.To, change.From)
+				}
+				b.P("COLLATE", to.V)
+				r.P("COLLATE", from.V)
+			default:
+				return fmt.Errorf("unexpected schema ModifyAttr: %T", change)
+			}
+		default:
+			return fmt.Errorf("unsupported ModifySchema change %T", change)
+		}
+	}
+	if b.Len() > 0 {
+		bs := Build("ALTER DATABASE").Ident(modify.S.Name)
+		rs := bs.Clone()
+		bs.WriteString(b.String())
+		rs.WriteString(r.String())
+		s.append(&migrate.Change{
+			Cmd:     bs.String(),
+			Reverse: rs.String(),
+			Source:  modify,
+			Comment: fmt.Sprintf("modify %q schema", modify.S.Name),
+		})
+	}
+	return nil
 }
 
 // addTable builds and appends the migrate.Change
diff --git a/sql/sqlite/diff.go b/sql/sqlite/diff.go
index ca5360fc..e0a68019 100644
--- a/sql/sqlite/diff.go
+++ b/sql/sqlite/diff.go
@@ -73,7 +73,15 @@ func (d *diff) typeChanged(from, to *schema.Column) (bool, error) {
 func (d *diff) defaultChanged(from, to *schema.Column) bool {
 	d1, ok1 := sqlx.DefaultValue(from)
 	d2, ok2 := sqlx.DefaultValue(to)
-	return ok1 != ok2 || d1 != d2
+	if ok1 != ok2 {
+		return true
+	}
+	if d1 == d2 {
+		return false
+	}
+	x1, err1 := sqlx.Unquote(d1)
+	x2, err2 := sqlx.Unquote(d2)
+	return err1 != nil || err2 != nil || x1 != x2
 }
 
 // IndexAttrChanged reports if the index attributes were changed.
diff --git a/sql/sqlite/migrate.go b/sql/sqlite/migrate.go
index 35b02028..0dfd91d2 100644
--- a/sql/sqlite/migrate.go
+++ b/sql/sqlite/migrate.go
@@ -87,13 +87,18 @@ func (s *state) plan(ctx context.Context, changes []schema.Change) (err error) {
 
 // addTable builds and executes the query for creating a table in a schema.
 func (s *state) addTable(ctx context.Context, add *schema.AddTable) error {
-	b := Build("CREATE TABLE").Ident(add.T.Name)
+	var (
+		errs []string
+		b    = Build("CREATE TABLE").Ident(add.T.Name)
+	)
 	if sqlx.Has(add.Extra, &schema.IfNotExists{}) {
 		b.P("IF NOT EXISTS")
 	}
 	b.Wrap(func(b *sqlx.Builder) {
 		b.MapComma(add.T.Columns, func(i int, b *sqlx.Builder) {
-			s.column(b, add.T.Columns[i])
+			if err := s.column(b, add.T.Columns[i]); err != nil {
+				errs = append(errs, err.Error())
+			}
 		})
 		// Primary keys with auto-increment are inlined on the column definition.
 		if pk := add.T.PrimaryKey; pk != nil && !autoincPK(pk) {
@@ -111,6 +116,9 @@ func (s *state) addTable(ctx context.Context, add *schema.AddTable) error {
 			}
 		}
 	})
+	if len(errs) > 0 {
+		return fmt.Errorf("create table %q: %s", add.T.Name, strings.Join(errs, ", "))
+	}
 	if p := (WithoutRowID{}); sqlx.Has(add.T.Attrs, &p) {
 		b.P("WITHOUT ROWID")
 	}
@@ -175,18 +183,23 @@ func (s *state) modifyTable(ctx context.Context, modify *schema.ModifyTable) err
 	return s.addIndexes(modify.T, indexes...)
 }
 
-func (s *state) column(b *sqlx.Builder, c *schema.Column) {
+func (s *state) column(b *sqlx.Builder, c *schema.Column) error {
 	b.Ident(c.Name).P(mustFormat(c.Type.Type))
 	if !c.Type.Null {
 		b.P("NOT")
 	}
 	b.P("NULL")
 	if c.Default != nil {
-		b.P("DEFAULT", defaultValue(c))
+		x, err := defaultValue(c)
+		if err != nil {
+			return err
+		}
+		b.P("DEFAULT", x)
 	}
 	if sqlx.Has(c.Attrs, &AutoIncrement{}) {
 		b.P("PRIMARY KEY AUTOINCREMENT")
 	}
+	return nil
 }
 
 func (s *state) addIndexes(t *schema.Table, indexes ...*schema.Index) error {
@@ -316,7 +329,11 @@ func (s *state) copyRows(from *schema.Table, to *schema.Table, changes []schema.
 			toC = append(toC, column.Name)
 			if !column.Type.Null && column.Default != nil && change.Change.Is(schema.ChangeNull|schema.ChangeDefault) {
 				fromC = append(fromC, fmt.Sprintf("IFNULL(`%[1]s`, ?) AS `%[1]s`", column.Name))
-				args = append(args, defaultValue(column))
+				x, err := defaultValue(column)
+				if err != nil {
+					return err
+				}
+				args = append(args, x)
 			} else {
 				fromC = append(fromC, column.Name)
 			}
@@ -352,7 +369,9 @@ func (s *state) alterTable(modify *schema.ModifyTable) error {
 			})
 		case *schema.AddColumn:
 			b := Build("ALTER TABLE").Ident(modify.T.Name).P("ADD COLUMN")
-			s.column(b, change.C)
+			if err := s.column(b, change.C); err != nil {
+				return err
+			}
 			// Unsupported reverse operation (DROP COLUMN).
 			s.append(&migrate.Change{
 				Cmd:     b.String(),
@@ -440,13 +459,13 @@ func Build(phrase string) *sqlx.Builder {
 	return b.P(phrase)
 }
 
-func defaultValue(c *schema.Column) string {
+func defaultValue(c *schema.Column) (string, error) {
 	switch x := c.Default.(type) {
 	case *schema.Literal:
-		return x.V
+		return sqlx.SingleQuote(x.V)
 	case *schema.RawExpr:
-		return x.X
+		return x.X, nil
 	default:
-		panic(fmt.Sprintf("unexpected default value type: %T", x))
+		return "", fmt.Errorf("unexpected default value type: %T", x)
 	}
 }


Test patch:
diff --git a/sql/mysql/diff_test.go b/sql/mysql/diff_test.go
index 859c5f5f..8006244a 100644
--- a/sql/mysql/diff_test.go
+++ b/sql/mysql/diff_test.go
@@ -383,8 +383,8 @@ func TestDiff_SchemaDiff(t *testing.T) {
 	changes, err = drv.SchemaDiff(from, to)
 	require.NoError(t, err)
 	require.EqualValues(t, []schema.Change{
-		&schema.ModifySchema{Changes: []schema.Change{&schema.ModifyAttr{From: from.Attrs[0], To: to.Attrs[0]}}},
-		&schema.ModifyTable{T: from.Tables[0], Changes: []schema.Change{&schema.AddColumn{C: to.Tables[0].Columns[0]}}},
+		&schema.ModifySchema{S: to, Changes: []schema.Change{&schema.ModifyAttr{From: from.Attrs[0], To: to.Attrs[0]}}},
+		&schema.ModifyTable{T: to.Tables[0], Changes: []schema.Change{&schema.AddColumn{C: to.Tables[0].Columns[0]}}},
 		&schema.DropTable{T: from.Tables[1]},
 		&schema.AddTable{T: to.Tables[1]},
 	}, changes)
@@ -449,8 +449,8 @@ func TestDiff_RealmDiff(t *testing.T) {
 	changes, err := drv.RealmDiff(from, to)
 	require.NoError(t, err)
 	require.EqualValues(t, []schema.Change{
-		&schema.ModifySchema{Changes: []schema.Change{&schema.ModifyAttr{From: from.Schemas[0].Attrs[0], To: to.Schemas[0].Attrs[0]}}},
-		&schema.ModifyTable{T: from.Schemas[0].Tables[0], Changes: []schema.Change{&schema.AddColumn{C: to.Schemas[0].Tables[0].Columns[0]}}},
+		&schema.ModifySchema{S: to.Schemas[0], Changes: []schema.Change{&schema.ModifyAttr{From: from.Schemas[0].Attrs[0], To: to.Schemas[0].Attrs[0]}}},
+		&schema.ModifyTable{T: to.Schemas[0].Tables[0], Changes: []schema.Change{&schema.AddColumn{C: to.Schemas[0].Tables[0].Columns[0]}}},
 		&schema.DropSchema{S: from.Schemas[1]},
 		&schema.AddSchema{S: to.Schemas[1]},
 		&schema.AddTable{T: to.Schemas[1].Tables[0]},
diff --git a/sql/mysql/migrate_test.go b/sql/mysql/migrate_test.go
index 0a089d3e..23e14d1e 100644
--- a/sql/mysql/migrate_test.go
+++ b/sql/mysql/migrate_test.go
@@ -185,6 +185,60 @@ func TestPlanChanges(t *testing.T) {
 				Changes:    []*migrate.Change{{Cmd: "CREATE DATABASE `test` CHARSET latin", Reverse: "DROP DATABASE `test`"}},
 			},
 		},
+		// Default database charset can be omitted.
+		{
+			input: []schema.Change{
+				&schema.AddSchema{S: schema.New("test").SetCharset("utf8")},
+			},
+			wantPlan: &migrate.Plan{
+				Reversible: true,
+				Changes:    []*migrate.Change{{Cmd: "CREATE DATABASE `test`", Reverse: "DROP DATABASE `test`"}},
+			},
+		},
+		// Add the default database charset on modify can be omitted.
+		{
+			input: []schema.Change{
+				&schema.ModifySchema{
+					S: schema.New("test").SetCharset("utf8"),
+					Changes: []schema.Change{
+						&schema.AddAttr{A: &schema.Charset{V: "utf8"}},
+					},
+				},
+			},
+			wantPlan: &migrate.Plan{
+				Reversible: true,
+			},
+		},
+		// Add custom charset.
+		{
+			input: []schema.Change{
+				&schema.ModifySchema{
+					S: schema.New("test").SetCharset("latin1"),
+					Changes: []schema.Change{
+						&schema.AddAttr{A: &schema.Charset{V: "latin1"}},
+					},
+				},
+			},
+			wantPlan: &migrate.Plan{
+				Reversible: true,
+				Changes:    []*migrate.Change{{Cmd: "ALTER DATABASE `test` CHARSET latin1", Reverse: "ALTER DATABASE `test`CHARSET utf8"}},
+			},
+		},
+		// Modify charset.
+		{
+			input: []schema.Change{
+				&schema.ModifySchema{
+					S: schema.New("test").SetCharset("utf8"),
+					Changes: []schema.Change{
+						&schema.ModifyAttr{From: &schema.Charset{V: "latin1"}, To: &schema.Charset{V: "utf8"}},
+					},
+				},
+			},
+			wantPlan: &migrate.Plan{
+				Reversible: true,
+				Changes:    []*migrate.Change{{Cmd: "ALTER DATABASE `test` CHARSET utf8", Reverse: "ALTER DATABASE `test`CHARSET latin1"}},
+			},
+		},
 		{
 			input: []schema.Change{
 				&schema.DropSchema{S: &schema.Schema{Name: "atlas", Attrs: []schema.Attr{&schema.Charset{V: "latin"}}}},
diff --git a/sql/postgres/diff_test.go b/sql/postgres/diff_test.go
index d7acaeae..57e549c1 100644
--- a/sql/postgres/diff_test.go
+++ b/sql/postgres/diff_test.go
@@ -312,7 +312,7 @@ func TestDiff_SchemaDiff(t *testing.T) {
 	changes, err := drv.SchemaDiff(from, to)
 	require.NoError(t, err)
 	require.EqualValues(t, []schema.Change{
-		&schema.ModifyTable{T: from.Tables[0], Changes: []schema.Change{&schema.AddColumn{C: to.Tables[0].Columns[0]}}},
+		&schema.ModifyTable{T: to.Tables[0], Changes: []schema.Change{&schema.AddColumn{C: to.Tables[0].Columns[0]}}},
 		&schema.DropTable{T: from.Tables[1]},
 		&schema.AddTable{T: to.Tables[1]},
 	}, changes)
diff --git a/sql/sqlite/diff_test.go b/sql/sqlite/diff_test.go
index 8b3b9424..7181b720 100644
--- a/sql/sqlite/diff_test.go
+++ b/sql/sqlite/diff_test.go
@@ -289,7 +289,7 @@ func TestDiff_SchemaDiff(t *testing.T) {
 	changes, err := drv.SchemaDiff(from, to)
 	require.NoError(t, err)
 	require.EqualValues(t, []schema.Change{
-		&schema.ModifyTable{T: from.Tables[0], Changes: []schema.Change{&schema.AddColumn{C: to.Tables[0].Columns[0]}}},
+		&schema.ModifyTable{T: to.Tables[0], Changes: []schema.Change{&schema.AddColumn{C: to.Tables[0].Columns[0]}}},
 		&schema.DropTable{T: from.Tables[1]},
 		&schema.AddTable{T: to.Tables[1]},
 	}, changes)