Repository: ariga/atlas
Pull request #394 (2022-01-11 21:29:55)

sql/mysql: allow dropping table attributes
Dropping charset or collations is applied using ModifyAttr.

Please see code comments.


Patch:
diff --git a/sql/mysql/diff.go b/sql/mysql/diff.go
index 758ab231..66ce2161 100644
--- a/sql/mysql/diff.go
+++ b/sql/mysql/diff.go
@@ -129,9 +129,13 @@ func (d *diff) collationChange(from, top, to []schema.Attr) schema.Change {
 			A: &toC,
 		}
 	case !toHas:
-		if !topHas || fromC.V != topC.V {
-			return &schema.DropAttr{
-				A: &fromC,
+		// There is no way to DROP a COLLATE that was configured on the table
+		// and it is not the default. Therefore, we use ModifyAttr and give it
+		// the inherited (and default) collation from schema or server.
+		if topHas && fromC.V != topC.V {
+			return &schema.ModifyAttr{
+				From: &fromC,
+				To:   &topC,
 			}
 		}
 	case fromC.V != toC.V:
@@ -154,9 +158,13 @@ func (d *diff) charsetChange(from, top, to []schema.Attr) schema.Change {
 			A: &toC,
 		}
 	case !toHas:
-		if !topHas || fromC.V != topC.V {
-			return &schema.DropAttr{
-				A: &fromC,
+		// There is no way to DROP a CHARSET that was configured on the table
+		// and it is not the default. Therefore, we use ModifyAttr and give it
+		// the inherited (and default) collation from schema or server.
+		if topHas && fromC.V != topC.V {
+			return &schema.ModifyAttr{
+				From: &fromC,
+				To:   &topC,
 			}
 		}
 	case fromC.V != toC.V:
diff --git a/sql/mysql/migrate.go b/sql/mysql/migrate.go
index 27e79c48..007f4b13 100644
--- a/sql/mysql/migrate.go
+++ b/sql/mysql/migrate.go
@@ -96,7 +96,7 @@ func (s *state) topLevel(changes []schema.Change) []schema.Change {
 				b.P("IF NOT EXISTS")
 			}
 			if a := (schema.Charset{}); sqlx.Has(c.S.Attrs, &a) {
-				b.P("CHARACTER SET", a.V)
+				b.P("CHARSET", a.V)
 			}
 			if a := (schema.Collation{}); sqlx.Has(c.S.Attrs, &a) {
 				b.P("COLLATE", a.V)
@@ -233,7 +233,7 @@ func (s *state) modifyTable(modify *schema.ModifyTable) error {
 				I: change.To,
 			})
 		case *schema.DropAttr:
-			return fmt.Errorf("unsupported change type: %T", change)
+			return fmt.Errorf("unsupported change type: %v", change.A)
 		default:
 			changes[1] = append(changes[1], change)
 		}
@@ -384,7 +384,7 @@ func (s *state) column(b *sqlx.Builder, t *schema.Table, c *schema.Column) error
 			// Define the charset explicitly
 			// in case it is not the default.
 			if s.character(t) != a.V {
-				b.P("CHARACTER SET", a.V)
+				b.P("CHARSET", a.V)
 			}
 		case *schema.Collation:
 			// Define the collation explicitly
@@ -470,7 +470,7 @@ func (s *state) tableAttr(b *sqlx.Builder, c schema.Change, attrs ...schema.Attr
 			// Ignore CHECK constraints as they are not real attributes,
 			// and handled on CREATE or ALTER.
 		case *schema.Charset:
-			b.P("CHARACTER SET", a.V)
+			b.P("CHARSET", a.V)
 		case *schema.Collation:
 			b.P("COLLATE", a.V)
 		case *schema.Comment:


Test patch:
diff --git a/sql/mysql/diff_test.go b/sql/mysql/diff_test.go
index 6be31b18..0b6f03c5 100644
--- a/sql/mysql/diff_test.go
+++ b/sql/mysql/diff_test.go
@@ -71,12 +71,13 @@ func TestDiff_TableDiff(t *testing.T) {
 			},
 		},
 		{
-			name: "drop collation",
-			from: &schema.Table{Name: "users", Schema: &schema.Schema{Name: "public"}, Attrs: []schema.Attr{&schema.Collation{V: "latin1"}}},
+			name: "drop collation means modify",
+			from: &schema.Table{Name: "users", Schema: &schema.Schema{Name: "public", Attrs: []schema.Attr{&schema.Collation{V: "utf8mb4_0900_ai_ci"}}}, Attrs: []schema.Attr{&schema.Collation{V: "utf8mb4_bin"}}},
 			to:   &schema.Table{Name: "users"},
 			wantChanges: []schema.Change{
-				&schema.DropAttr{
-					A: &schema.Collation{V: "latin1"},
+				&schema.ModifyAttr{
+					From: &schema.Collation{V: "utf8mb4_bin"},
+					To:   &schema.Collation{V: "utf8mb4_0900_ai_ci"},
 				},
 			},
 		},
@@ -102,12 +103,13 @@ func TestDiff_TableDiff(t *testing.T) {
 			},
 		},
 		{
-			name: "drop charset",
-			from: &schema.Table{Name: "users", Schema: &schema.Schema{Name: "public"}, Attrs: []schema.Attr{&schema.Charset{V: "hebrew"}}},
+			name: "drop charset means modify",
+			from: &schema.Table{Name: "users", Schema: &schema.Schema{Name: "public", Attrs: []schema.Attr{&schema.Charset{V: "hebrew"}}}, Attrs: []schema.Attr{&schema.Charset{V: "hebrew_bin"}}},
 			to:   &schema.Table{Name: "users"},
 			wantChanges: []schema.Change{
-				&schema.DropAttr{
-					A: &schema.Charset{V: "hebrew"},
+				&schema.ModifyAttr{
+					From: &schema.Charset{V: "hebrew_bin"},
+					To:   &schema.Charset{V: "hebrew"},
 				},
 			},
 		},
diff --git a/sql/mysql/migrate_test.go b/sql/mysql/migrate_test.go
index 07204ae2..32b1838f 100644
--- a/sql/mysql/migrate_test.go
+++ b/sql/mysql/migrate_test.go
@@ -19,7 +19,7 @@ import (
 func TestMigrate_ApplyChanges(t *testing.T) {
 	migrate, mk, err := newMigrate("8.0.13")
 	require.NoError(t, err)
-	mk.ExpectExec(sqltest.Escape("CREATE DATABASE `test` CHARACTER SET latin")).
+	mk.ExpectExec(sqltest.Escape("CREATE DATABASE `test` CHARSET latin")).
 		WillReturnResult(sqlmock.NewResult(0, 1))
 	mk.ExpectExec(sqltest.Escape("DROP DATABASE `atlas`")).
 		WillReturnResult(sqlmock.NewResult(0, 1))
@@ -181,7 +181,7 @@ func TestPlanChanges(t *testing.T) {
 			},
 			plan: &migrate.Plan{
 				Reversible: true,
-				Changes:    []*migrate.Change{{Cmd: "CREATE DATABASE `test` CHARACTER SET latin", Reverse: "DROP DATABASE `test`"}},
+				Changes:    []*migrate.Change{{Cmd: "CREATE DATABASE `test` CHARSET latin", Reverse: "DROP DATABASE `test`"}},
 			},
 		},
 		{
@@ -209,7 +209,7 @@ func TestPlanChanges(t *testing.T) {
 			},
 			plan: &migrate.Plan{
 				Reversible: true,
-				Changes:    []*migrate.Change{{Cmd: "CREATE TABLE `posts` (`id` bigint NOT NULL AUTO_INCREMENT, `text` text NULL, `uuid` char(36) NULL CHARACTER SET utf8mb4 COLLATE utf8mb4_bin, PRIMARY KEY (`id`))", Reverse: "DROP TABLE `posts`"}},
+				Changes:    []*migrate.Change{{Cmd: "CREATE TABLE `posts` (`id` bigint NOT NULL AUTO_INCREMENT, `text` text NULL, `uuid` char(36) NULL CHARSET utf8mb4 COLLATE utf8mb4_bin, PRIMARY KEY (`id`))", Reverse: "DROP TABLE `posts`"}},
 			},
 		},
 		{
@@ -245,7 +245,7 @@ func TestPlanChanges(t *testing.T) {
 			},
 			plan: &migrate.Plan{
 				Reversible: true,
-				Changes:    []*migrate.Change{{Cmd: "CREATE TABLE `posts` (`id` bigint NOT NULL AUTO_INCREMENT, `text` text NULL, `ch` char NOT NULL, PRIMARY KEY (`id`), INDEX `text_prefix` (`text` (100) DESC), CONSTRAINT `id_nonzero` CHECK (`id` > 0)) CHARACTER SET utf8mb4 COLLATE utf8mb4_bin COMMENT \"posts comment\" COMPRESSION=\"ZLIB\" AUTO_INCREMENT 100", Reverse: "DROP TABLE `posts`"}},
+				Changes:    []*migrate.Change{{Cmd: "CREATE TABLE `posts` (`id` bigint NOT NULL AUTO_INCREMENT, `text` text NULL, `ch` char NOT NULL, PRIMARY KEY (`id`), INDEX `text_prefix` (`text` (100) DESC), CONSTRAINT `id_nonzero` CHECK (`id` > 0)) CHARSET utf8mb4 COLLATE utf8mb4_bin COMMENT \"posts comment\" COMPRESSION=\"ZLIB\" AUTO_INCREMENT 100", Reverse: "DROP TABLE `posts`"}},
 			},
 		},
 		{