Repository: ariga/atlas
Pull request #1246 (2022-10-30 12:56:24)

sql/sqlcheck: add check for foreign-keys' drops


Patch:
diff --git a/doc/md/lint/analyzers.md b/doc/md/lint/analyzers.md
index 9cc0c182..3f408951 100644
--- a/doc/md/lint/analyzers.md
+++ b/doc/md/lint/analyzers.md
@@ -103,12 +103,13 @@ The following schema change checks are provided by Atlas:
 | [MF102](#MF102)                    | Modifying non-unique index to unique                                        |
 | [MF103](#MF103)                    | Adding a non-nullable column to an existing table                           |
 | [MF104](#MF104)                    | Modifying a nullable column to non-nullable                                 |
+| **CD1**                            | Constraint deletion changes                                                 |
+| [CD101](#CD101)                    | Foreign-key constraint was dropped                                          |
 | **MY**                             | MySQL and MariaDB specific checks                                           |
 | [MY101](#MY101)                    | Adding a non-nullable column without a `DEFAULT` value to an existing table |
 | **LT**                             | SQLite specific checks                                                      |
 | [LT101](#LT101)                    | Modifying a nullable column to non-nullable without a `DEFAULT` value       |
 
-
 #### DS101 {#DS101}
 
 Destructive change that is reported when a database schema was dropped. For example:
@@ -173,6 +174,14 @@ UPDATE t SET c = 0 WHERE c IS NULL;
 ALTER TABLE t MODIFY COLUMN c int NOT NULL;
 ```
 
+#### CD101 {#CD101}
+
+Constraint deletion is reported when a foreign-key constraint was dropped. For example:
+
+```sql
+ALTER TABLE pets DROP CONSTRAINT owner_id;
+```
+
 #### MY101 {#MY101}
 
 Adding a non-nullable column to a table without a `DEFAULT` value implicitly sets existing rows with the column
diff --git a/sql/mysql/mysqlcheck/mysqlcheck.go b/sql/mysql/mysqlcheck/mysqlcheck.go
index 82149395..c01c891f 100644
--- a/sql/mysql/mysqlcheck/mysqlcheck.go
+++ b/sql/mysql/mysqlcheck/mysqlcheck.go
@@ -14,6 +14,7 @@ import (
 	"ariga.io/atlas/sql/mysql"
 	"ariga.io/atlas/sql/schema"
 	"ariga.io/atlas/sql/sqlcheck"
+	"ariga.io/atlas/sql/sqlcheck/condrop"
 	"ariga.io/atlas/sql/sqlcheck/datadepend"
 	"ariga.io/atlas/sql/sqlcheck/destructive"
 )
@@ -149,12 +150,16 @@ func init() {
 		if err != nil {
 			return nil, err
 		}
+		cd, err := condrop.New(r)
+		if err != nil {
+			return nil, err
+		}
 		dd, err := datadepend.New(r, datadepend.Handler{
 			AddNotNull: addNotNull,
 		})
 		if err != nil {
 			return nil, err
 		}
-		return []sqlcheck.Analyzer{ds, dd}, nil
+		return []sqlcheck.Analyzer{ds, dd, cd}, nil
 	})
 }
diff --git a/sql/postgres/postgrescheck/postgrescheck.go b/sql/postgres/postgrescheck/postgrescheck.go
index 268901c5..bfc8dbf1 100644
--- a/sql/postgres/postgrescheck/postgrescheck.go
+++ b/sql/postgres/postgrescheck/postgrescheck.go
@@ -10,6 +10,7 @@ import (
 	"ariga.io/atlas/schemahcl"
 	"ariga.io/atlas/sql/postgres"
 	"ariga.io/atlas/sql/sqlcheck"
+	"ariga.io/atlas/sql/sqlcheck/condrop"
 	"ariga.io/atlas/sql/sqlcheck/datadepend"
 	"ariga.io/atlas/sql/sqlcheck/destructive"
 )
@@ -36,9 +37,13 @@ func init() {
 		if err != nil {
 			return nil, err
 		}
+		cd, err := condrop.New(r)
+		if err != nil {
+			return nil, err
+		}
 		dd, err := datadepend.New(r, datadepend.Handler{
 			AddNotNull: addNotNull,
 		})
-		return []sqlcheck.Analyzer{ds, dd}, nil
+		return []sqlcheck.Analyzer{ds, dd, cd}, nil
 	})
 }
diff --git a/sql/sqlcheck/condrop/condrop.go b/sql/sqlcheck/condrop/condrop.go
new file mode 100644
index 00000000..2696e525
--- /dev/null
+++ b/sql/sqlcheck/condrop/condrop.go
@@ -0,0 +1,85 @@
+// Copyright 2021-present The Atlas Authors. All rights reserved.
+// This source code is licensed under the Apache 2.0 license found
+// in the LICENSE file in the root directory of this source tree.
+
+package condrop
+
+import (
+	"context"
+	"errors"
+	"fmt"
+
+	"ariga.io/atlas/schemahcl"
+	"ariga.io/atlas/sql/internal/sqlx"
+	"ariga.io/atlas/sql/schema"
+	"ariga.io/atlas/sql/sqlcheck"
+)
+
+// Analyzer checks for constraint-dropping changes.
+type Analyzer struct {
+	sqlcheck.Options
+}
+
+// New creates a new constraint-dropping Analyzer with the given options.
+func New(r *schemahcl.Resource) (*Analyzer, error) {
+	az := &Analyzer{}
+	if r, ok := r.Resource(az.Name()); ok {
+		if err := r.As(&az.Options); err != nil {
+			return nil, fmt.Errorf("sql/sqlcheck: parsing datadepend check options: %w", err)
+		}
+	}
+	return az, nil
+}
+
+// List of codes.
+var (
+	codeDropF = sqlcheck.Code("CD101")
+)
+
+// Name of the analyzer. Implements the sqlcheck.NamedAnalyzer interface.
+func (*Analyzer) Name() string {
+	return "condrop"
+}
+
+// Analyze implements sqlcheck.Analyzer.
+func (a *Analyzer) Analyze(_ context.Context, p *sqlcheck.Pass) error {
+	var diags []sqlcheck.Diagnostic
+	for _, sc := range p.File.Changes {
+		for _, c := range sc.Changes {
+			c, ok := c.(*schema.ModifyTable)
+			if !ok {
+				continue
+			}
+			for i := range c.Changes {
+				d, ok := c.Changes[i].(*schema.DropForeignKey)
+				if !ok {
+					continue
+				}
+				dropC := func() bool {
+					for i := range d.F.Columns {
+						if schema.Changes(c.Changes).IndexDropColumn(d.F.Columns[i].Name) != -1 {
+							return true
+						}
+					}
+					return false
+				}()
+				// If none of the foreign-key columns were dropped.
+				if !dropC {
+					diags = append(diags, sqlcheck.Diagnostic{
+						Code: codeDropF,
+						Pos:  sc.Stmt.Pos,
+						Text: fmt.Sprintf("Dropping foreign-key constraint %q", d.F.Symbol),
+					})
+				}
+			}
+		}
+	}
+	if len(diags) > 0 {
+		const reportText = "constraint deletion detected"
+		p.Reporter.WriteReport(sqlcheck.Report{Text: reportText, Diagnostics: diags})
+		if sqlx.V(a.Error) {
+			return errors.New(reportText)
+		}
+	}
+	return nil
+}
diff --git a/sql/sqlite/sqlitecheck/sqlitecheck.go b/sql/sqlite/sqlitecheck/sqlitecheck.go
index 532078f5..f945d028 100644
--- a/sql/sqlite/sqlitecheck/sqlitecheck.go
+++ b/sql/sqlite/sqlitecheck/sqlitecheck.go
@@ -9,11 +9,11 @@ import (
 	"fmt"
 	"strings"
 
-	"ariga.io/atlas/sql/migrate"
-
 	"ariga.io/atlas/schemahcl"
+	"ariga.io/atlas/sql/migrate"
 	"ariga.io/atlas/sql/schema"
 	"ariga.io/atlas/sql/sqlcheck"
+	"ariga.io/atlas/sql/sqlcheck/condrop"
 	"ariga.io/atlas/sql/sqlcheck/datadepend"
 	"ariga.io/atlas/sql/sqlcheck/destructive"
 	"ariga.io/atlas/sql/sqlite"
@@ -57,6 +57,10 @@ func init() {
 		if err != nil {
 			return nil, err
 		}
+		cd, err := condrop.New(r)
+		if err != nil {
+			return nil, err
+		}
 		dd, err := datadepend.New(r, datadepend.Handler{
 			AddNotNull:    addNotNull,
 			ModifyNotNull: modifyNotNull,
@@ -102,7 +106,7 @@ func init() {
 				p.File.Changes = changes
 				return nil
 			}),
-			ds, dd,
+			ds, dd, cd,
 		}, nil
 	})
 }


Test patch:
diff --git a/internal/integration/testdata/mysql/cli-migrate-lint-condrop.txt b/internal/integration/testdata/mysql/cli-migrate-lint-condrop.txt
new file mode 100644
index 00000000..cd10336d
--- /dev/null
+++ b/internal/integration/testdata/mysql/cli-migrate-lint-condrop.txt
@@ -0,0 +1,30 @@
+only mysql8
+
+# Expect to log only the connected schema.
+! atlas migrate lint --dir file://migrations1 --dev-url URL --latest=1 > got.txt
+cmp got.txt migrations1.txt
+
+-- migrations1/1.sql --
+CREATE TABLE users (id int, u1 int, PRIMARY KEY(id));
+CREATE TABLE pets (id int, owner_id int, PRIMARY KEY(id), CONSTRAINT pet_owner_id FOREIGN KEY(owner_id) REFERENCES users(id));
+CREATE TABLE cars (id int, owner_id int, PRIMARY KEY(id), CONSTRAINT car_owner_id FOREIGN KEY(owner_id) REFERENCES users(id));
+
+-- migrations1/2.sql --
+/* Rename table mixed with rename column is not a destructive change. */
+ALTER TABLE users RENAME TO user, RENAME COLUMN u1 TO u2;
+
+-- migrations1/3.sql --
+ALTER TABLE user ADD COLUMN u1 int, DROP COLUMN u2;
+ALTER TABLE cars DROP CONSTRAINT car_owner_id, DROP COLUMN owner_id;
+ALTER TABLE pets DROP CONSTRAINT pet_owner_id;
+
+-- migrations1.txt --
+3.sql: destructive changes detected:
+
+	L1: Dropping non-virtual column "u2"
+	L2: Dropping non-virtual column "owner_id"
+
+3.sql: constraint deletion detected:
+
+	L3: Dropping foreign-key constraint "pet_owner_id"
+
diff --git a/sql/sqlcheck/condrop/condrop_test.go b/sql/sqlcheck/condrop/condrop_test.go
new file mode 100644
index 00000000..9e47a242
--- /dev/null
+++ b/sql/sqlcheck/condrop/condrop_test.go
@@ -0,0 +1,76 @@
+// Copyright 2021-present The Atlas Authors. All rights reserved.
+// This source code is licensed under the Apache 2.0 license found
+// in the LICENSE file in the root directory of this source tree.
+
+package condrop_test
+
+import (
+	"context"
+	"testing"
+
+	"ariga.io/atlas/schemahcl"
+	"ariga.io/atlas/sql/migrate"
+	"ariga.io/atlas/sql/schema"
+	"ariga.io/atlas/sql/sqlcheck"
+	"ariga.io/atlas/sql/sqlcheck/condrop"
+	"ariga.io/atlas/sql/sqlclient"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestAnalyzer_DropForeignKey(t *testing.T) {
+	var (
+		report sqlcheck.Report
+		pass   = &sqlcheck.Pass{
+			Dev: &sqlclient.Client{Name: "mysql"},
+			File: &sqlcheck.File{
+				File: testFile{name: "1.sql"},
+				Changes: []*sqlcheck.Change{
+					{
+						Stmt: &migrate.Stmt{
+							Text: "ALTER TABLE `pets`",
+						},
+						Changes: []schema.Change{
+							&schema.ModifyTable{
+								T: schema.NewTable("pets").
+									SetSchema(schema.New("test")),
+								Changes: schema.Changes{
+									&schema.DropColumn{
+										C: schema.NewColumn("c"),
+									},
+									&schema.DropForeignKey{
+										F: schema.NewForeignKey("owner_id").
+											AddColumns(schema.NewColumn("owner_id")),
+									},
+									&schema.DropForeignKey{
+										F: schema.NewForeignKey("c").
+											AddColumns(schema.NewColumn("c"), schema.NewColumn("d")),
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			Reporter: sqlcheck.ReportWriterFunc(func(r sqlcheck.Report) {
+				report = r
+			}),
+		}
+	)
+	az, err := condrop.New(&schemahcl.Resource{})
+	require.NoError(t, err)
+	err = az.Analyze(context.Background(), pass)
+	require.NoError(t, err)
+	require.Len(t, report.Diagnostics, 1)
+	require.Equal(t, "constraint deletion detected", report.Text)
+	require.Equal(t, `Dropping foreign-key constraint "owner_id"`, report.Diagnostics[0].Text)
+}
+
+type testFile struct {
+	name string
+	migrate.File
+}
+
+func (t testFile) Name() string {
+	return t.name
+}
diff --git a/sql/sqlite/sqlitecheck/sqlitecheck_test.go b/sql/sqlite/sqlitecheck/sqlitecheck_test.go
index 36715954..b7fd50f5 100644
--- a/sql/sqlite/sqlitecheck/sqlitecheck_test.go
+++ b/sql/sqlite/sqlitecheck/sqlitecheck_test.go
@@ -126,7 +126,7 @@ func TestDetectModifyTable(t *testing.T) {
 	)
 	azs, err := sqlcheck.AnalyzerFor(sqlite.DriverName, nil)
 	require.NoError(t, err)
-	require.Len(t, azs, 3)
+	require.Len(t, azs, 4)
 	require.NoError(t, azs[0].Analyze(context.Background(), pass))
 	err = azs[1].Analyze(context.Background(), pass)
 	require.EqualError(t, err, "destructive changes detected")