Repository: ariga/atlas
Pull request #454 (2022-01-20 17:35:47)

cmd/atlas: support canary version: update canary logic


Patch:
diff --git a/cmd/action/version.go b/cmd/action/version.go
index 5391ba37..edf78ec0 100644
--- a/cmd/action/version.go
+++ b/cmd/action/version.go
@@ -27,13 +27,13 @@ func init() {
 
 // parse returns a user facing version and release notes url
 func parse(version string) (string, string) {
+	u := "https://github.com/ariga/atlas/releases/tag/latest"
 	if ok := semver.IsValid(version); !ok {
-		v := "- development"
-		s := strings.Split(version, "_")
-		if len(s) != 0 && s[len(s)-1] == "canary" {
-			v = version
-		}
-		return v, "https://github.com/ariga/atlas/releases/tag/latest"
+		return "- development", u
 	}
-	return version, fmt.Sprintf("https://github.com/ariga/atlas/releases/tag/%s", version)
+	s := strings.Split(version, "-")
+	if len(s) != 0 && s[len(s)-1] != "canary" {
+		u = fmt.Sprintf("https://github.com/ariga/atlas/releases/tag/%s", version)
+	}
+	return version, u
 }
diff --git a/sql/internal/sqlx/diff.go b/sql/internal/sqlx/diff.go
index 4a51c431..5a1e26c6 100644
--- a/sql/internal/sqlx/diff.go
+++ b/sql/internal/sqlx/diff.go
@@ -439,7 +439,7 @@ func CommentDiff(from, to []schema.Attr) schema.Change {
 
 // 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 {
+func CheckDiff(from, to *schema.Table, compare ...func(c1, c2 *schema.Check) bool) []schema.Change {
 	var changes []schema.Change
 	// Drop or modify checks.
 	for _, c1 := range checks(from.Attrs) {
@@ -448,7 +448,7 @@ func CheckDiff(from, to *schema.Table, compare func(c1, c2 *schema.Check) bool)
 			changes = append(changes, &schema.DropCheck{
 				C: c1,
 			})
-		case compare(c1, c2):
+		case len(compare) == 1 && !compare[0](c1, c2):
 			changes = append(changes, &schema.ModifyCheck{
 				From: c1,
 				To:   c2,
diff --git a/sql/mysql/diff.go b/sql/mysql/diff.go
index b39b77ee..0866c62e 100644
--- a/sql/mysql/diff.go
+++ b/sql/mysql/diff.go
@@ -56,7 +56,7 @@ func (d *diff) TableAttrDiff(from, to *schema.Table) ([]schema.Change, error) {
 	// using "MODIFY COLUMN".
 	var checks []schema.Change
 	for _, c := range sqlx.CheckDiff(from, to, func(c1, c2 *schema.Check) bool {
-		return c1.Expr != c2.Expr || sqlx.Has(c1.Attrs, &Enforced{}) != sqlx.Has(c2.Attrs, &Enforced{})
+		return enforced(c1.Attrs) == enforced(c2.Attrs)
 	}) {
 		drop, ok := c.(*schema.DropCheck)
 		if !ok || !strings.HasPrefix(drop.C.Expr, "json_valid") {
@@ -263,6 +263,15 @@ func indexType(attr []schema.Attr) *IndexType {
 	return t
 }
 
+// enforced returns the ENFORCED attribute for the CHECK
+// constraint. A CHECK is ENFORCED if not state otherwise.
+func enforced(attr []schema.Attr) bool {
+	if e := (Enforced{}); sqlx.Has(attr, &e) {
+		return e.V
+	}
+	return true
+}
+
 // noChange describes a zero change.
 var noChange struct{ schema.Change }
 
diff --git a/sql/mysql/inspect.go b/sql/mysql/inspect.go
index 07de9c1a..f9996b78 100644
--- a/sql/mysql/inspect.go
+++ b/sql/mysql/inspect.go
@@ -398,14 +398,10 @@ func (i *inspect) checks(ctx context.Context, t *schema.Table) error {
 			Name: name.String,
 			Expr: unescape(clause.String),
 		}
-		if enforced.String != "NO" {
-			check.Attrs = append(check.Attrs, &Enforced{})
-		}
-		t.Attrs = append(t.Attrs, check)
-		// In MariaDB, JSON is an alias to LONGTEXT. For versions >= 10.4.3, the CHARSET and COLLATE set to utf8mb4
-		// and a CHECK constraint is automatically created for the column as well (i.e. JSON_VALID(`<C>`)). However,
-		// we expect tools like Atlas and Ent to manually add this CHECK for older versions of MariaDB.
 		if i.mariadb() {
+			// In MariaDB, JSON is an alias to LONGTEXT. For versions >= 10.4.3, the CHARSET and COLLATE set to utf8mb4
+			// and a CHECK constraint is automatically created for the column as well (i.e. JSON_VALID(`<C>`)). However,
+			// we expect tools like Atlas and Ent to manually add this CHECK for older versions of MariaDB.
 			c, ok := t.Column(check.Name)
 			if ok && c.Type.Raw == TypeLongText && check.Expr == fmt.Sprintf("json_valid(`%s`)", c.Name) {
 				c.Type.Raw = TypeJSON
@@ -414,8 +410,13 @@ func (i *inspect) checks(ctx context.Context, t *schema.Table) error {
 				// as they are valid only for character types.
 				c.UnsetCharset().UnsetCollation()
 			}
+		} else if enforced.String == "NO" {
+			// The ENFORCED attribute is not supported by MariaDB.
+			// Also, skip adding it in case the CHECK is ENFORCED,
+			// as the default is ENFORCED if not state otherwise.
+			check.Attrs = append(check.Attrs, &Enforced{V: false})
 		}
-
+		t.Attrs = append(t.Attrs, check)
 	}
 	return rows.Err()
 }
@@ -725,9 +726,9 @@ type (
 	}
 
 	// Enforced attribute defines the ENFORCED flag for CHECK constraint.
-	// Similar to AUTO_INCREMENT, checks with this attributes are enforced.
 	Enforced struct {
 		schema.Attr
+		V bool // V indicates if the CHECK is enforced or not.
 	}
 
 	// The DisplayWidth represents a display width of an integer type.
diff --git a/sql/mysql/sqlspec.go b/sql/mysql/sqlspec.go
index 14edb225..eef8db23 100644
--- a/sql/mysql/sqlspec.go
+++ b/sql/mysql/sqlspec.go
@@ -99,9 +99,7 @@ func convertCheck(spec *sqlspec.Check) (*schema.Check, error) {
 		if err != nil {
 			return nil, err
 		}
-		if b {
-			c.AddAttrs(&Enforced{})
-		}
+		c.AddAttrs(&Enforced{V: b})
 	}
 	return c, nil
 }
diff --git a/sql/postgres/diff.go b/sql/postgres/diff.go
index a1556295..f8301d5d 100644
--- a/sql/postgres/diff.go
+++ b/sql/postgres/diff.go
@@ -32,7 +32,7 @@ func (d *diff) TableAttrDiff(from, to *schema.Table) ([]schema.Change, error) {
 		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{})
+		return sqlx.Has(c1.Attrs, &NoInherit{}) == sqlx.Has(c2.Attrs, &NoInherit{})
 	})...), nil
 }
 
diff --git a/sql/sqlite/diff.go b/sql/sqlite/diff.go
index 2f6539d1..3be6de8d 100644
--- a/sql/sqlite/diff.go
+++ b/sql/sqlite/diff.go
@@ -36,9 +36,7 @@ func (d *diff) TableAttrDiff(from, to *schema.Table) ([]schema.Change, error) {
 			A: &WithoutRowID{},
 		})
 	}
-	return append(changes, sqlx.CheckDiff(from, to, func(c1, c2 *schema.Check) bool {
-		return c1.Expr != c2.Expr
-	})...), nil
+	return append(changes, sqlx.CheckDiff(from, to)...), nil
 }
 
 // ColumnChange returns the schema changes (if any) for migrating one column to the other.


Test patch:
diff --git a/cmd/action/internal/update/update_test.go b/cmd/action/internal/update/update_test.go
index 127d43ec..84b1eb46 100644
--- a/cmd/action/internal/update/update_test.go
+++ b/cmd/action/internal/update/update_test.go
@@ -61,6 +61,14 @@ func TestCheckForUpdate(t *testing.T) {
 			shouldNotify:   true,
 			message:        fmt.Sprintf("A new version of Atlas is available (v2.2.3):\n%s", url),
 		},
+		{
+			name:           "no store fetch and update newer patch version - canary",
+			currentVersion: "v1.2.3-6539f2704b5d-canary",
+			latestVersion:  "v1.2.4",
+			latestRelease:  &LatestRelease{Version: "v2.2.3", URL: url},
+			shouldNotify:   true,
+			message:        fmt.Sprintf("A new version of Atlas is available (v2.2.3):\n%s", url),
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
diff --git a/internal/integration/integration_test.go b/internal/integration/integration_test.go
index 4b61db76..d25c9c67 100644
--- a/internal/integration/integration_test.go
+++ b/internal/integration/integration_test.go
@@ -229,11 +229,11 @@ func TestCLI_Version(t *testing.T) {
 			name: "canary",
 			cmd: exec.Command("go", "run",
 				"-ldflags",
-				"-X ariga.io/atlas/cmd/action.version=v0.3.0_6539f2704b5d_canary",
+				"-X ariga.io/atlas/cmd/action.version=v0.3.0-6539f2704b5d-canary",
 				"ariga.io/atlas/cmd/atlas",
 				"version",
 			),
-			expected: "atlas CLI version v0.3.0_6539f2704b5d_canary\nhttps://github.com/ariga/atlas/releases/tag/latest\n",
+			expected: "atlas CLI version v0.3.0-6539f2704b5d-canary\nhttps://github.com/ariga/atlas/releases/tag/latest\n",
 		},
 	}
 	for _, tt := range tests {
diff --git a/internal/integration/mysql_test.go b/internal/integration/mysql_test.go
index eedcc580..98d5e288 100644
--- a/internal/integration/mysql_test.go
+++ b/internal/integration/mysql_test.go
@@ -921,7 +921,7 @@ create table atlas_types_sanity
 						return []schema.Attr{
 							&schema.Charset{V: "latin1"},
 							&schema.Collation{V: "latin1_swedish_ci"},
-							&schema.Check{Name: "tJSON", Expr: "json_valid(`tJSON`)", Attrs: []schema.Attr{&mysql.Enforced{}}},
+							&schema.Check{Name: "tJSON", Expr: "json_valid(`tJSON`)"},
 						}
 					}
 					return []schema.Attr{
diff --git a/sql/mysql/diff_test.go b/sql/mysql/diff_test.go
index d5985be2..c3cf8c40 100644
--- a/sql/mysql/diff_test.go
+++ b/sql/mysql/diff_test.go
@@ -146,12 +146,12 @@ func TestDiff_TableDiff(t *testing.T) {
 		},
 		{
 			name: "modify check",
-			from: &schema.Table{Name: "t1", Schema: &schema.Schema{Name: "public"}, Attrs: []schema.Attr{&schema.Check{Name: "users_chk1_c1", Expr: "(`c1` <>_latin1\\'foo\\')"}}},
-			to:   &schema.Table{Name: "t1", Attrs: []schema.Attr{&schema.Check{Name: "users_chk1_c1", Expr: "(`c1` <>_latin1\\'foo\\')", Attrs: []schema.Attr{&Enforced{}}}}},
+			from: &schema.Table{Name: "t1", Schema: &schema.Schema{Name: "public"}, Attrs: []schema.Attr{&schema.Check{Name: "users_chk1_c1", Expr: "(`c1` <>_latin1\\'foo\\')", Attrs: []schema.Attr{&Enforced{V: false}}}}},
+			to:   &schema.Table{Name: "t1", Attrs: []schema.Attr{&schema.Check{Name: "users_chk1_c1", Expr: "(`c1` <>_latin1\\'foo\\')", Attrs: []schema.Attr{&Enforced{V: true}}}}},
 			wantChanges: []schema.Change{
 				&schema.ModifyCheck{
-					From: &schema.Check{Name: "users_chk1_c1", Expr: "(`c1` <>_latin1\\'foo\\')"},
-					To:   &schema.Check{Name: "users_chk1_c1", Expr: "(`c1` <>_latin1\\'foo\\')", Attrs: []schema.Attr{&Enforced{}}},
+					From: &schema.Check{Name: "users_chk1_c1", Expr: "(`c1` <>_latin1\\'foo\\')", Attrs: []schema.Attr{&Enforced{V: false}}},
+					To:   &schema.Check{Name: "users_chk1_c1", Expr: "(`c1` <>_latin1\\'foo\\')", Attrs: []schema.Attr{&Enforced{V: true}}},
 				},
 			},
 		},
diff --git a/sql/mysql/inspect_test.go b/sql/mysql/inspect_test.go
index cb2517f3..8195cfd6 100644
--- a/sql/mysql/inspect_test.go
+++ b/sql/mysql/inspect_test.go
@@ -606,7 +606,7 @@ func TestDriver_InspectTable(t *testing.T) {
 					{Name: "c1", Type: &schema.ColumnType{Raw: "int", Type: &schema.IntegerType{T: "int"}}},
 				}
 				require.EqualValues(columns, t.Columns)
-				require.EqualValues([]schema.Attr{&CreateStmt{S: "CREATE TABLE users()"}, &schema.Check{Name: "users_chk_1", Expr: "(`c6` <>_latin1\\'foo\\'s\\')", Attrs: []schema.Attr{&Enforced{}}}}, t.Attrs)
+				require.EqualValues([]schema.Attr{&CreateStmt{S: "CREATE TABLE users()"}, &schema.Check{Name: "users_chk_1", Expr: "(`c6` <>_latin1\\'foo\\'s\\')"}}, t.Attrs)
 			},
 		},
 	}
diff --git a/sql/mysql/sqlspec_test.go b/sql/mysql/sqlspec_test.go
index 002e471e..7a6af0ca 100644
--- a/sql/mysql/sqlspec_test.go
+++ b/sql/mysql/sqlspec_test.go
@@ -57,6 +57,10 @@ table "table" {
 		expr     = "price1 <> price2"
 		enforced = true
 	}
+	check {
+		expr     = "price2 <> price1"
+		enforced = false
+	}
 	comment = "table comment"
 }
 
@@ -133,7 +137,11 @@ table "accounts" {
 				},
 				&schema.Check{
 					Expr:  "price1 <> price2",
-					Attrs: []schema.Attr{&Enforced{}},
+					Attrs: []schema.Attr{&Enforced{V: true}},
+				},
+				&schema.Check{
+					Expr:  "price2 <> price1",
+					Attrs: []schema.Attr{&Enforced{V: false}},
 				},
 				&schema.Comment{Text: "table comment"},
 			},
diff --git a/sql/postgres/diff_test.go b/sql/postgres/diff_test.go
index 57e549c1..c84abf81 100644
--- a/sql/postgres/diff_test.go
+++ b/sql/postgres/diff_test.go
@@ -89,17 +89,6 @@ func TestDiff_TableDiff(t *testing.T) {
 				},
 			},
 		},
-		{
-			name: "modify check",
-			from: &schema.Table{Name: "t1", Schema: &schema.Schema{Name: "public"}, Attrs: []schema.Attr{&schema.Check{Name: "t1_c1_check", Expr: "(c1 > 1)"}}},
-			to:   &schema.Table{Name: "t1", Attrs: []schema.Attr{&schema.Check{Name: "t1_c1_check", Expr: "(c1 > 1)", Attrs: []schema.Attr{&NoInherit{}}}}},
-			wantChanges: []schema.Change{
-				&schema.ModifyCheck{
-					From: &schema.Check{Name: "t1_c1_check", Expr: "(c1 > 1)"},
-					To:   &schema.Check{Name: "t1_c1_check", Expr: "(c1 > 1)", Attrs: []schema.Attr{&NoInherit{}}},
-				},
-			},
-		},
 		{
 			name: "add comment",
 			from: &schema.Table{Name: "t1", Schema: &schema.Schema{Name: "public"}},
diff --git a/sql/sqlite/diff_test.go b/sql/sqlite/diff_test.go
index 7181b720..90841504 100644
--- a/sql/sqlite/diff_test.go
+++ b/sql/sqlite/diff_test.go
@@ -95,17 +95,6 @@ func TestDiff_TableDiff(t *testing.T) {
 				},
 			},
 		},
-		{
-			name: "modify check",
-			from: &schema.Table{Name: "t1", Attrs: []schema.Attr{&schema.Check{Name: "t1_c1_check", Expr: "(c1 > 1)"}}},
-			to:   &schema.Table{Name: "t1", Attrs: []schema.Attr{&schema.Check{Name: "t1_c1_check", Expr: "(c1 > 2)"}}},
-			wantChanges: []schema.Change{
-				&schema.ModifyCheck{
-					From: &schema.Check{Name: "t1_c1_check", Expr: "(c1 > 1)"},
-					To:   &schema.Check{Name: "t1_c1_check", Expr: "(c1 > 2)"},
-				},
-			},
-		},
 		func() testcase {
 			var (
 				from = &schema.Table{