Repository: ariga/atlas
Pull request #408 (2022-01-13 21:23:07)

sql: support diffing table comments


Patch:
diff --git a/sql/internal/sqlx/diff.go b/sql/internal/sqlx/diff.go
index c6de5f41..0eec8760 100644
--- a/sql/internal/sqlx/diff.go
+++ b/sql/internal/sqlx/diff.go
@@ -8,6 +8,8 @@ import (
 	"fmt"
 	"reflect"
 	"sort"
+	"strconv"
+	"strings"
 
 	"ariga.io/atlas/sql/schema"
 )
@@ -349,6 +351,39 @@ func (e UnsupportedTypeError) Error() string {
 	return fmt.Sprintf("unsupported type %T", e.Type)
 }
 
+// CommentDiff computes the comment diff between the 2 attribute list.
+// Note that, the implementation relies on the fact that both PostgreSQL
+// and MySQL treat empty comment as "no comment" and a way to clear comments.
+func CommentDiff(from, to []schema.Attr) schema.Change {
+	var fromC, toC schema.Comment
+	switch fromHas, toHas := Has(from, &fromC), Has(to, &toC); {
+	case !fromHas && !toHas:
+	case !fromHas && toC.Text != "":
+		return &schema.AddAttr{
+			A: &toC,
+		}
+	case !toHas:
+		// In MySQL, there is no way to DROP a comment. Instead, setting it to empty ('')
+		// will remove it from INFORMATION_SCHEMA. We use the same approach in PostgreSQL,
+		// because comments can be dropped either by setting them to NULL or empty string.
+		// See: postgres/backend/commands/comment.c#CreateComments.
+		return &schema.ModifyAttr{
+			From: &fromC,
+			To:   &toC,
+		}
+	default:
+		v1, err1 := Unquote(fromC.Text)
+		v2, err2 := Unquote(toC.Text)
+		if err1 == nil && err2 == nil && v1 != v2 {
+			return &schema.ModifyAttr{
+				From: &fromC,
+				To:   &toC,
+			}
+		}
+	}
+	return nil
+}
+
 // CheckDiff computes the change diff between the 2 tables. A compare
 // function is provided to check if a Check object was modified.
 func CheckDiff(from, to *schema.Table, compare func(c1, c2 *schema.Check) bool) []schema.Change {
@@ -412,3 +447,15 @@ func similarCheck(attrs []schema.Attr, c *schema.Check) (*schema.Check, bool) {
 	}
 	return nil, false
 }
+
+// Unquote single or double quotes.
+func Unquote(s string) (string, error) {
+	switch {
+	case IsQuoted(s, '"'):
+		return strconv.Unquote(s)
+	case IsQuoted(s, '\''):
+		return strings.ReplaceAll(s[1:len(s)-1], "''", "'"), nil
+	default:
+		return s, nil
+	}
+}
diff --git a/sql/mysql/diff.go b/sql/mysql/diff.go
index 66ce2161..600e0738 100644
--- a/sql/mysql/diff.go
+++ b/sql/mysql/diff.go
@@ -38,6 +38,9 @@ func (d *diff) TableAttrDiff(from, to *schema.Table) []schema.Change {
 	if change := d.autoIncChange(from.Attrs, to.Attrs); change != noChange {
 		changes = append(changes, change)
 	}
+	if change := sqlx.CommentDiff(from.Attrs, to.Attrs); change != nil {
+		changes = append(changes, change)
+	}
 	if change := d.charsetChange(from.Attrs, from.Schema.Attrs, to.Attrs); change != noChange {
 		changes = append(changes, change)
 	}
@@ -102,7 +105,7 @@ func (*diff) ReferenceChanged(from, to schema.ReferenceOption) bool {
 }
 
 // Normalize implements the sqlx.Normalizer interface.
-func (d *diff) Normalize(from, to *schema.Table) {
+func (*diff) Normalize(from, to *schema.Table) {
 	indexes := make([]*schema.Index, 0, len(from.Indexes))
 	for _, idx := range from.Indexes {
 		// MySQL requires that foreign key columns be indexed; Therefore, if the child
@@ -120,7 +123,7 @@ func (d *diff) Normalize(from, to *schema.Table) {
 
 // collationChange returns the schema change for migrating the collation if
 // it was changed and its not the default attribute inherited from its parent.
-func (d *diff) collationChange(from, top, to []schema.Attr) schema.Change {
+func (*diff) collationChange(from, top, to []schema.Attr) schema.Change {
 	var fromC, topC, toC schema.Collation
 	switch fromHas, topHas, toHas := sqlx.Has(from, &fromC), sqlx.Has(top, &topC), sqlx.Has(to, &toC); {
 	case !fromHas && !toHas:
@@ -149,7 +152,7 @@ func (d *diff) collationChange(from, top, to []schema.Attr) schema.Change {
 
 // charsetChange returns the schema change for migrating the collation if
 // it was changed and its not the default attribute inherited from its parent.
-func (d *diff) charsetChange(from, top, to []schema.Attr) schema.Change {
+func (*diff) charsetChange(from, top, to []schema.Attr) schema.Change {
 	var fromC, topC, toC schema.Charset
 	switch fromHas, topHas, toHas := sqlx.Has(from, &fromC), sqlx.Has(top, &topC), sqlx.Has(to, &toC); {
 	case !fromHas && !toHas:
@@ -178,7 +181,7 @@ func (d *diff) charsetChange(from, top, to []schema.Attr) schema.Change {
 
 // autoIncChange returns the schema change for changing the AUTO_INCREMENT
 // attribute in case it is not the default.
-func (d *diff) autoIncChange(from, to []schema.Attr) schema.Change {
+func (*diff) autoIncChange(from, to []schema.Attr) schema.Change {
 	var fromA, toA AutoIncrement
 	// The table is empty and AUTO_INCREMENT was not configured. This can happen
 	// because older versions of MySQL (< 8.0) stored the AUTO_INCREMENT counter
@@ -344,8 +347,8 @@ func (d *diff) equalsIntValues(x1, x2 string) bool {
 // equalsStringValues report if the 2 string default values are
 // equal after dropping their quotes.
 func equalsStringValues(x1, x2 string) bool {
-	a, err1 := unquote(x1)
-	b, err2 := unquote(x2)
+	a, err1 := sqlx.Unquote(x1)
+	b, err2 := sqlx.Unquote(x2)
 	return a == b && err1 == nil && err2 == nil
 }
 
diff --git a/sql/mysql/migrate.go b/sql/mysql/migrate.go
index 111a2818..3052608c 100644
--- a/sql/mysql/migrate.go
+++ b/sql/mysql/migrate.go
@@ -613,14 +613,3 @@ func quote(s string) string {
 	}
 	return strconv.Quote(s)
 }
-
-func unquote(s string) (string, error) {
-	switch {
-	case sqlx.IsQuoted(s, '"'):
-		return strconv.Unquote(s)
-	case sqlx.IsQuoted(s, '\''):
-		return strings.ReplaceAll(s[1:len(s)-1], "''", "'"), nil
-	default:
-		return s, nil
-	}
-}
diff --git a/sql/postgres/diff.go b/sql/postgres/diff.go
index 5a2d0b85..a2d251ec 100644
--- a/sql/postgres/diff.go
+++ b/sql/postgres/diff.go
@@ -25,9 +25,13 @@ func (d *diff) SchemaAttrDiff(_, _ *schema.Schema) []schema.Change {
 
 // TableAttrDiff returns a changeset for migrating table attributes from one state to the other.
 func (d *diff) TableAttrDiff(from, to *schema.Table) []schema.Change {
-	return sqlx.CheckDiff(from, to, func(c1, c2 *schema.Check) bool {
+	var changes []schema.Change
+	if change := sqlx.CommentDiff(from.Attrs, to.Attrs); change != nil {
+		changes = append(changes, change)
+	}
+	return append(changes, sqlx.CheckDiff(from, to, func(c1, c2 *schema.Check) bool {
 		return c1.Expr != c2.Expr || sqlx.Has(c1.Attrs, &NoInherit{}) != sqlx.Has(c2.Attrs, &NoInherit{})
-	})
+	})...)
 }
 
 // ColumnChange returns the schema changes (if any) for migrating one column to the other.


Test patch:
diff --git a/sql/mysql/diff_test.go b/sql/mysql/diff_test.go
index 0b6f03c5..859c5f5f 100644
--- a/sql/mysql/diff_test.go
+++ b/sql/mysql/diff_test.go
@@ -155,6 +155,38 @@ func TestDiff_TableDiff(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "add comment",
+			from: &schema.Table{Name: "t1", Schema: &schema.Schema{Name: "public"}},
+			to:   &schema.Table{Name: "t1", Attrs: []schema.Attr{&schema.Comment{Text: "t1"}}},
+			wantChanges: []schema.Change{
+				&schema.AddAttr{
+					A: &schema.Comment{Text: "t1"},
+				},
+			},
+		},
+		{
+			name: "drop comment",
+			from: &schema.Table{Name: "t1", Schema: &schema.Schema{Name: "public"}, Attrs: []schema.Attr{&schema.Comment{Text: "t1"}}},
+			to:   &schema.Table{Name: "t1"},
+			wantChanges: []schema.Change{
+				&schema.ModifyAttr{
+					From: &schema.Comment{Text: "t1"},
+					To:   &schema.Comment{Text: ""},
+				},
+			},
+		},
+		{
+			name: "modify comment",
+			from: &schema.Table{Name: "t1", Schema: &schema.Schema{Name: "public"}, Attrs: []schema.Attr{&schema.Comment{Text: "t1"}}},
+			to:   &schema.Table{Name: "t1", Attrs: []schema.Attr{&schema.Comment{Text: "t1!"}}},
+			wantChanges: []schema.Change{
+				&schema.ModifyAttr{
+					From: &schema.Comment{Text: "t1"},
+					To:   &schema.Comment{Text: "t1!"},
+				},
+			},
+		},
 		func() testcase {
 			var (
 				from = &schema.Table{
diff --git a/sql/postgres/diff_test.go b/sql/postgres/diff_test.go
index 5959a77a..d7acaeae 100644
--- a/sql/postgres/diff_test.go
+++ b/sql/postgres/diff_test.go
@@ -100,6 +100,38 @@ func TestDiff_TableDiff(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "add comment",
+			from: &schema.Table{Name: "t1", Schema: &schema.Schema{Name: "public"}},
+			to:   &schema.Table{Name: "t1", Attrs: []schema.Attr{&schema.Comment{Text: "t1"}}},
+			wantChanges: []schema.Change{
+				&schema.AddAttr{
+					A: &schema.Comment{Text: "t1"},
+				},
+			},
+		},
+		{
+			name: "drop comment",
+			from: &schema.Table{Name: "t1", Schema: &schema.Schema{Name: "public"}, Attrs: []schema.Attr{&schema.Comment{Text: "t1"}}},
+			to:   &schema.Table{Name: "t1"},
+			wantChanges: []schema.Change{
+				&schema.ModifyAttr{
+					From: &schema.Comment{Text: "t1"},
+					To:   &schema.Comment{Text: ""},
+				},
+			},
+		},
+		{
+			name: "modify comment",
+			from: &schema.Table{Name: "t1", Schema: &schema.Schema{Name: "public"}, Attrs: []schema.Attr{&schema.Comment{Text: "t1"}}},
+			to:   &schema.Table{Name: "t1", Attrs: []schema.Attr{&schema.Comment{Text: "t1!"}}},
+			wantChanges: []schema.Change{
+				&schema.ModifyAttr{
+					From: &schema.Comment{Text: "t1"},
+					To:   &schema.Comment{Text: "t1!"},
+				},
+			},
+		},
 		func() testcase {
 			var (
 				from = &schema.Table{