Repository: ariga/atlas
Pull request #889 (2022-06-23 07:53:42)

sql/sqlcheck: skip temporary resources in destructive analyzer


Patch:
diff --git a/sql/sqlcheck/analyzers.go b/sql/sqlcheck/analyzers.go
new file mode 100644
index 00000000..451f6659
--- /dev/null
+++ b/sql/sqlcheck/analyzers.go
@@ -0,0 +1,97 @@
+// 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 sqlcheck
+
+import (
+	"context"
+	"fmt"
+	"strings"
+	"sync"
+
+	"ariga.io/atlas/sql/internal/sqlx"
+	"ariga.io/atlas/sql/schema"
+)
+
+// Destructive checks destructive changes.
+var Destructive = &driverAware{
+	run: func(ctx context.Context, diags []Diagnostic, p *Pass) error {
+		for _, sc := range p.File.Changes {
+			for _, c := range sc.Changes {
+				switch c := c.(type) {
+				case *schema.DropSchema:
+					if p.File.SchemaSpan(c.S) != SpanTemporary {
+						var text string
+						switch n := len(c.S.Tables); {
+						case n == 0:
+							text = fmt.Sprintf("Dropping schema %q", c.S.Name)
+						case n == 1:
+							text = fmt.Sprintf("Dropping non-empty schema %q with 1 table", c.S.Name)
+						case n > 1:
+							text = fmt.Sprintf("Dropping non-empty schema %q with %d tables", c.S.Name, n)
+						}
+						diags = append(diags, Diagnostic{Pos: sc.Pos, Text: text})
+					}
+				case *schema.DropTable:
+					if p.File.SchemaSpan(c.T.Schema) != SpanDropped && p.File.TableSpan(c.T) != SpanTemporary {
+						diags = append(diags, Diagnostic{
+							Pos:  sc.Pos,
+							Text: fmt.Sprintf("Dropping table %q", c.T.Name),
+						})
+					}
+				case *schema.ModifyTable:
+					for i := range c.Changes {
+						d, ok := c.Changes[i].(*schema.DropColumn)
+						if !ok || p.File.ColumnSpan(c.T, d.C) == SpanTemporary {
+							continue
+						}
+						if g := (schema.GeneratedExpr{}); !sqlx.Has(d.C.Attrs, &g) || strings.ToUpper(g.Type) != "VIRTUAL" {
+							diags = append(diags, Diagnostic{
+								Pos:  sc.Pos,
+								Text: fmt.Sprintf("Dropping non-virtual column %q", d.C.Name),
+							})
+						}
+					}
+				}
+			}
+		}
+		if len(diags) > 0 {
+			p.Reporter.WriteReport(Report{
+				Text:        fmt.Sprintf("Destructive changes detected in file %s", p.File.Name()),
+				Diagnostics: diags,
+			})
+		}
+		return nil
+	},
+}
+
+// driverAware is a type of analyzer that allows registering driver-level diagnostic functions.
+type driverAware struct {
+	run     func(context.Context, []Diagnostic, *Pass) error
+	mu      sync.RWMutex
+	drivers map[string]func(context.Context, *Pass) ([]Diagnostic, error)
+}
+
+// Register registers driver-level run function to extend the analyzer.
+func (a *driverAware) Register(name string, run func(context.Context, *Pass) ([]Diagnostic, error)) {
+	a.mu.Lock()
+	if a.drivers == nil {
+		a.drivers = make(map[string]func(context.Context, *Pass) ([]Diagnostic, error))
+	}
+	a.drivers[name] = run
+	a.mu.Unlock()
+}
+
+// Analyze implements the Analyzer interface.
+func (a *driverAware) Analyze(ctx context.Context, p *Pass) error {
+	var diags []Diagnostic
+	if run, ok := a.drivers[p.Dev.Name]; ok {
+		d, err := run(ctx, p)
+		if err != nil {
+			return err
+		}
+		diags = d
+	}
+	return a.run(ctx, diags, p)
+}
diff --git a/sql/sqlcheck/sqlcheck.go b/sql/sqlcheck/sqlcheck.go
index 6a35f4f5..b73aba96 100644
--- a/sql/sqlcheck/sqlcheck.go
+++ b/sql/sqlcheck/sqlcheck.go
@@ -6,10 +6,7 @@ package sqlcheck
 
 import (
 	"context"
-	"fmt"
-	"sync"
 
-	"ariga.io/atlas/sql/internal/sqlx"
 	"ariga.io/atlas/sql/migrate"
 	"ariga.io/atlas/sql/schema"
 	"ariga.io/atlas/sql/sqlclient"
@@ -31,7 +28,7 @@ type (
 		// Dev is a driver-specific environment used to execute analysis work.
 		Dev *sqlclient.Client
 
-		// Report reports a analysis reports.
+		// Report reports analysis reports.
 		Reporter ReportWriter
 	}
 
@@ -46,6 +43,9 @@ type (
 		// in case of a file that contains exactly two statements, and the first
 		// statement is reverted by the one after it, the Sum is nil.
 		Sum schema.Changes
+
+		// schema spans. lazily initialized.
+		spans map[string]*schemaSpan
 	}
 
 	// A Change in a migration file.
@@ -94,74 +94,118 @@ func (f ReportWriterFunc) WriteReport(r Report) {
 	f(r)
 }
 
-// Destructive checks destructive changes.
-var Destructive = &driverAware{
-	run: func(ctx context.Context, diags []Diagnostic, p *Pass) error {
-		for _, sc := range p.File.Changes {
-			for _, c := range sc.Changes {
-				switch c := c.(type) {
-				case *schema.DropSchema:
-					diags = append(diags, Diagnostic{
-						Pos:  sc.Pos,
-						Text: fmt.Sprintf("Dropping schema %q", c.S.Name),
-					})
-				case *schema.DropTable:
-					diags = append(diags, Diagnostic{
-						Pos:  sc.Pos,
-						Text: fmt.Sprintf("Dropping table %q", c.T.Name),
-					})
-				case *schema.ModifyTable:
-					for i := range c.Changes {
-						d, ok := c.Changes[i].(*schema.DropColumn)
-						if !ok {
-							continue
-						}
-						if g := (schema.GeneratedExpr{}); !sqlx.Has(d.C.Attrs, &g) || g.Type != "VIRTUAL" {
-							diags = append(diags, Diagnostic{
-								Pos:  sc.Pos,
-								Text: fmt.Sprintf("Dropping non-virtual column %q", d.C.Name),
-							})
-						}
+// ResourceSpan describes the lifespan of a resource
+// in perspective to the migration file.
+type ResourceSpan uint
+
+const (
+	// SpanUnknown describes unknown lifespan.
+	// e.g. resource may exist before this file.
+	SpanUnknown ResourceSpan = iota
+
+	// SpanAdded describes that a span of
+	// a resource was started in this file.
+	SpanAdded
+
+	// SpanDropped describes that a span of
+	// a resource was ended in this file.
+	SpanDropped
+
+	// SpanTemporary indicates that a resource lifetime
+	// was started and ended in this file (CREATE and DROP).
+	SpanTemporary = SpanAdded | SpanDropped
+)
+
+// SchemaSpan returns the span information for the schema.
+func (f *File) SchemaSpan(s *schema.Schema) ResourceSpan {
+	return f.schemaSpan(s).state
+}
+
+// TableSpan returns the span information for the table.
+func (f *File) TableSpan(t *schema.Table) ResourceSpan {
+	return f.tableSpan(t).state
+}
+
+// ColumnSpan returns the span information for the column.
+func (f *File) ColumnSpan(t *schema.Table, c *schema.Column) ResourceSpan {
+	return f.tableSpan(t).columns[c.Name]
+}
+
+// IndexSpan returns the span information for the span.
+func (f *File) IndexSpan(t *schema.Table, i *schema.Index) ResourceSpan {
+	return f.tableSpan(t).indexes[i.Name]
+}
+
+type (
+	// schemaSpan holds the span structure of a schema.
+	schemaSpan struct {
+		state  ResourceSpan
+		tables map[string]*tableSpan
+	}
+	// schemaSpan holds the span structure of a table.
+	tableSpan struct {
+		state   ResourceSpan
+		columns map[string]ResourceSpan
+		indexes map[string]ResourceSpan
+	}
+)
+
+func (f *File) loadSpans() {
+	f.spans = make(map[string]*schemaSpan)
+	for _, sc := range f.Changes {
+		for _, c := range sc.Changes {
+			switch c := c.(type) {
+			case *schema.AddSchema:
+				f.schemaSpan(c.S).state = SpanAdded
+			case *schema.DropSchema:
+				f.schemaSpan(c.S).state |= SpanDropped
+			case *schema.AddTable:
+				span := f.tableSpan(c.T)
+				span.state = SpanAdded
+				for _, column := range c.T.Columns {
+					span.columns[column.Name] = SpanAdded
+				}
+				for _, idx := range c.T.Indexes {
+					span.indexes[idx.Name] = SpanAdded
+				}
+			case *schema.DropTable:
+				f.tableSpan(c.T).state |= SpanDropped
+			case *schema.ModifyTable:
+				span := f.tableSpan(c.T)
+				for _, c1 := range c.Changes {
+					switch c1 := c1.(type) {
+					case *schema.AddColumn:
+						span.columns[c1.C.Name] = SpanAdded
+					case *schema.DropColumn:
+						span.columns[c1.C.Name] |= SpanDropped
+					case *schema.AddIndex:
+						span.indexes[c1.I.Name] = SpanAdded
+					case *schema.DropIndex:
+						span.indexes[c1.I.Name] |= SpanDropped
 					}
 				}
 			}
 		}
-		if len(diags) > 0 {
-			p.Reporter.WriteReport(Report{
-				Text:        fmt.Sprintf("Destructive changes detected in file %s", p.File.Name()),
-				Diagnostics: diags,
-			})
-		}
-		return nil
-	},
-}
-
-// driverAware is a type of analyzer that allows registering driver-level diagnostic functions.
-type driverAware struct {
-	run     func(context.Context, []Diagnostic, *Pass) error
-	mu      sync.RWMutex
-	drivers map[string]func(context.Context, *Pass) ([]Diagnostic, error)
+	}
 }
 
-// Register registers driver-level run function to extend the analyzer.
-func (a *driverAware) Register(name string, run func(context.Context, *Pass) ([]Diagnostic, error)) {
-	a.mu.Lock()
-	if a.drivers == nil {
-		a.drivers = make(map[string]func(context.Context, *Pass) ([]Diagnostic, error))
+func (f *File) schemaSpan(s *schema.Schema) *schemaSpan {
+	if f.spans == nil {
+		f.loadSpans()
+	}
+	if f.spans[s.Name] == nil {
+		f.spans[s.Name] = &schemaSpan{tables: make(map[string]*tableSpan)}
 	}
-	a.drivers[name] = run
-	a.mu.Unlock()
+	return f.spans[s.Name]
 }
 
-// Analyze implements the Analyzer interface.
-func (a *driverAware) Analyze(ctx context.Context, p *Pass) error {
-	var diags []Diagnostic
-	if run, ok := a.drivers[p.Dev.Name]; ok {
-		d, err := run(ctx, p)
-		if err != nil {
-			return err
+func (f *File) tableSpan(t *schema.Table) *tableSpan {
+	span := f.schemaSpan(t.Schema)
+	if span.tables[t.Name] == nil {
+		span.tables[t.Name] = &tableSpan{
+			columns: make(map[string]ResourceSpan),
+			indexes: make(map[string]ResourceSpan),
 		}
-		diags = d
 	}
-	return a.run(ctx, diags, p)
+	return f.spans[t.Schema.Name].tables[t.Name]
 }


Test patch:
diff --git a/sql/sqlcheck/analyzers_test.go b/sql/sqlcheck/analyzers_test.go
new file mode 100644
index 00000000..8b9d6968
--- /dev/null
+++ b/sql/sqlcheck/analyzers_test.go
@@ -0,0 +1,224 @@
+// 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 sqlcheck_test
+
+import (
+	"context"
+	"testing"
+
+	"ariga.io/atlas/sql/migrate"
+	"ariga.io/atlas/sql/schema"
+	"ariga.io/atlas/sql/sqlcheck"
+	"ariga.io/atlas/sql/sqlclient"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestDestructive_DropTable(t *testing.T) {
+	var (
+		report *sqlcheck.Report
+		pass   = &sqlcheck.Pass{
+			Dev: &sqlclient.Client{},
+			File: &sqlcheck.File{
+				File: testFile{name: "1.sql"},
+				Changes: []*sqlcheck.Change{
+					{
+						Stmt: "DROP TABLE `users`",
+						Changes: schema.Changes{
+							&schema.DropTable{
+								T: schema.NewTable("users").
+									SetSchema(schema.New("test")),
+							},
+						},
+					},
+					{
+						Stmt: "DROP TABLE `posts`",
+						Changes: schema.Changes{
+							&schema.DropTable{
+								T: schema.NewTable("posts").
+									SetSchema(schema.New("test")),
+							},
+						},
+					},
+					{
+						Stmt: "CREATE TABLE `posts`",
+						Changes: schema.Changes{
+							&schema.AddTable{
+								T: schema.NewTable("posts").
+									SetSchema(schema.New("test")),
+							},
+						},
+					},
+				},
+			},
+			Reporter: sqlcheck.ReportWriterFunc(func(r sqlcheck.Report) {
+				report = &r
+			}),
+		}
+	)
+	err := sqlcheck.Destructive.Analyze(context.Background(), pass)
+	require.NoError(t, err)
+	require.Equal(t, `Destructive changes detected in file 1.sql`, report.Text)
+	require.Len(t, report.Diagnostics, 2)
+	require.Equal(t, `Dropping table "users"`, report.Diagnostics[0].Text)
+	require.Equal(t, `Dropping table "posts"`, report.Diagnostics[1].Text)
+}
+
+func TestDestructive_SkipTemporaryTable(t *testing.T) {
+	var (
+		report *sqlcheck.Report
+		pass   = &sqlcheck.Pass{
+			Dev: &sqlclient.Client{},
+			File: &sqlcheck.File{
+				File: testFile{name: "1.sql"},
+				Changes: []*sqlcheck.Change{
+					{
+						Stmt: "DROP TABLE `users`",
+						Changes: schema.Changes{
+							&schema.AddTable{
+								T: schema.NewTable("users").
+									SetSchema(schema.New("test")),
+							},
+							&schema.DropTable{
+								T: schema.NewTable("users").
+									SetSchema(schema.New("test")),
+							},
+						},
+					},
+				},
+			},
+			Reporter: sqlcheck.ReportWriterFunc(func(r sqlcheck.Report) {
+				report = &r
+			}),
+		}
+	)
+	err := sqlcheck.Destructive.Analyze(context.Background(), pass)
+	require.NoError(t, err)
+	require.Nil(t, report, "no report")
+}
+
+func TestDestructive_DropSchema(t *testing.T) {
+	var (
+		report *sqlcheck.Report
+		pass   = &sqlcheck.Pass{
+			Dev: &sqlclient.Client{},
+			File: &sqlcheck.File{
+				File: testFile{name: "1.sql"},
+				Changes: []*sqlcheck.Change{
+					{
+						Stmt: "DROP SCHEMA `test`",
+						Changes: schema.Changes{
+							&schema.DropSchema{
+								S: schema.New("test").
+									AddTables(
+										schema.NewTable("users"),
+										schema.NewTable("orders"),
+									),
+							},
+						},
+					},
+					{
+						Stmt: "DROP SCHEMA `market`",
+						Changes: schema.Changes{
+							&schema.DropSchema{
+								S: schema.New("market"),
+							},
+						},
+					},
+					{
+						Stmt: "CREATE DATABASE `market`",
+						Changes: schema.Changes{
+							&schema.AddSchema{
+								S: schema.New("market"),
+							},
+						},
+					},
+				},
+			},
+			Reporter: sqlcheck.ReportWriterFunc(func(r sqlcheck.Report) {
+				report = &r
+			}),
+		}
+	)
+	err := sqlcheck.Destructive.Analyze(context.Background(), pass)
+	require.NoError(t, err)
+	require.Equal(t, `Destructive changes detected in file 1.sql`, report.Text)
+	require.Len(t, report.Diagnostics, 2)
+	require.Equal(t, `Dropping non-empty schema "test" with 2 tables`, report.Diagnostics[0].Text)
+	require.Equal(t, `Dropping schema "market"`, report.Diagnostics[1].Text)
+}
+
+func TestDestructive_Register(t *testing.T) {
+	sqlcheck.Destructive.Register("mysql", func(ctx context.Context, p *sqlcheck.Pass) (diags []sqlcheck.Diagnostic, _ error) {
+		for _, cs := range p.File.Changes {
+			for _, c := range cs.Changes {
+				_, ok := c.(*schema.ModifySchema)
+				if !ok {
+					continue
+				}
+				// A fake driver-level diagnostic.
+				diags = append(diags, sqlcheck.Diagnostic{Text: "fake diagnostic", Pos: cs.Pos})
+			}
+		}
+		return
+	})
+	var (
+		report sqlcheck.Report
+		pass   = &sqlcheck.Pass{
+			Dev: &sqlclient.Client{Name: "mysql"},
+			File: &sqlcheck.File{
+				File: testFile{name: "1.sql"},
+				Changes: []*sqlcheck.Change{
+					{
+						Stmt: "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").
+											SetGeneratedExpr(&schema.GeneratedExpr{Type: "STORED"}),
+									},
+								},
+							},
+						},
+					},
+					{
+						Stmt: "ALTER TABLE `pets`",
+						Changes: []schema.Change{
+							&schema.ModifySchema{
+								S: schema.New("test"),
+								Changes: schema.Changes{
+									&schema.ModifyAttr{
+										From: &schema.Charset{V: "utf8"},
+										To:   &schema.Charset{V: "latin1"},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			Reporter: sqlcheck.ReportWriterFunc(func(r sqlcheck.Report) {
+				report = r
+			}),
+		}
+	)
+	err := sqlcheck.Destructive.Analyze(context.Background(), pass)
+	require.NoError(t, err)
+	require.Len(t, report.Diagnostics, 2)
+	require.Equal(t, "fake diagnostic", report.Diagnostics[0].Text)
+	require.Equal(t, `Dropping non-virtual column "c"`, report.Diagnostics[1].Text)
+}
+
+type testFile struct {
+	name string
+	migrate.File
+}
+
+func (t testFile) Name() string {
+	return t.name
+}
diff --git a/sql/sqlcheck/sqlcheck_test.go b/sql/sqlcheck/sqlcheck_test.go
deleted file mode 100644
index 615c0e76..00000000
--- a/sql/sqlcheck/sqlcheck_test.go
+++ /dev/null
@@ -1,91 +0,0 @@
-// 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 sqlcheck_test
-
-import (
-	"context"
-	"testing"
-
-	"ariga.io/atlas/sql/migrate"
-	"ariga.io/atlas/sql/schema"
-	"ariga.io/atlas/sql/sqlcheck"
-	"ariga.io/atlas/sql/sqlclient"
-
-	"github.com/stretchr/testify/require"
-)
-
-func TestDestructive(t *testing.T) {
-	sqlcheck.Destructive.Register("mysql", func(ctx context.Context, p *sqlcheck.Pass) (diags []sqlcheck.Diagnostic, _ error) {
-		for _, cs := range p.File.Changes {
-			for _, c := range cs.Changes {
-				_, ok := c.(*schema.ModifyTable)
-				if !ok {
-					continue
-				}
-				// A fake driver-level diagnostic.
-				diags = append(diags, sqlcheck.Diagnostic{Text: "modify table", Pos: cs.Pos})
-			}
-		}
-		return
-	})
-	var (
-		report sqlcheck.Report
-		pass   = &sqlcheck.Pass{
-			Dev: &sqlclient.Client{Name: "mysql"},
-			File: &sqlcheck.File{
-				File: testFile{name: "1.sql"},
-				Changes: []*sqlcheck.Change{
-					{
-						Stmt: "DROP DATABASE `test`",
-						Changes: schema.Changes{
-							&schema.DropSchema{S: schema.New("test")},
-						},
-					},
-					{
-						Stmt: "DROP TABLE `users`",
-						Changes: schema.Changes{
-							&schema.DropTable{T: schema.NewTable("users")},
-						},
-					},
-				},
-			},
-			Reporter: sqlcheck.ReportWriterFunc(func(r sqlcheck.Report) {
-				report = r
-			}),
-		}
-	)
-	err := sqlcheck.Destructive.Analyze(context.Background(), pass)
-	require.NoError(t, err)
-	require.Equal(t, `Destructive changes detected in file 1.sql`, report.Text)
-	require.Len(t, report.Diagnostics, 2)
-
-	pass.File.Changes = append(pass.File.Changes, &sqlcheck.Change{
-		Stmt: "MODIFY TABLE `pets`",
-		Changes: schema.Changes{
-			&schema.ModifyTable{
-				T: schema.NewTable("pets"),
-				Changes: schema.Changes{
-					&schema.ModifyColumn{
-						Change: schema.ChangeType,
-						From:   schema.NewIntColumn("c", "int"),
-						To:     schema.NewDecimalColumn("c", "decimal"),
-					},
-				},
-			},
-		},
-	})
-	err = sqlcheck.Destructive.Analyze(context.Background(), pass)
-	require.NoError(t, err)
-	require.Len(t, report.Diagnostics, 3)
-}
-
-type testFile struct {
-	name string
-	migrate.File
-}
-
-func (t testFile) Name() string {
-	return t.name
-}