Repository: ariga/atlas
Pull request #2563 (2024-02-21 10:01:31)

sql/migrate: add info about checksum mismatch reason


Patch:
diff --git a/cmd/atlas/internal/cmdapi/migrate.go b/cmd/atlas/internal/cmdapi/migrate.go
index a8de4c04..64b7aef0 100644
--- a/cmd/atlas/internal/cmdapi/migrate.go
+++ b/cmd/atlas/internal/cmdapi/migrate.go
@@ -172,7 +172,7 @@ func migrateApplyRun(cmd *cobra.Command, args []string, flags migrateApplyFlags,
 		return err
 	}
 	if err := migrate.Validate(dir); err != nil {
-		printChecksumError(cmd)
+		printChecksumError(cmd, err)
 		return err
 	}
 	// Open a client to the database.
@@ -841,7 +841,7 @@ func migrateImportCmd() *cobra.Command {
 					return err
 				}
 				if err = migrate.Validate(d); err != nil && !errors.Is(err, migrate.ErrChecksumNotFound) {
-					printChecksumError(cmd)
+					printChecksumError(cmd, err)
 					return err
 				}
 				return nil
@@ -1340,7 +1340,8 @@ files are executed on the connected database in order to validate SQL semantics.
 				if err := dirFormatBC(flags.dirFormat, &flags.dirURL); err != nil {
 					return err
 				}
-				return checkDir(cmd, flags.dirURL, false)
+				err := checkDir(cmd, flags.dirURL, false)
+				return err
 			},
 			RunE: RunE(func(cmd *cobra.Command, args []string) error {
 				return migrateValidateRun(cmd, args, flags)
@@ -1601,23 +1602,24 @@ func checkDir(cmd *cobra.Command, url string, create bool) error {
 		return err
 	}
 	if err = migrate.Validate(d); err != nil {
-		printChecksumError(cmd)
+		printChecksumError(cmd, err)
 		return err
 	}
 	return nil
 }
 
-func printChecksumError(cmd *cobra.Command) {
-	fmt.Fprintf(cmd.OutOrStderr(), `You have a checksum error in your migration directory.
-This happens if you manually create or edit a migration file.
-Please check your migration files and run
-
-'atlas migrate hash'
-
-to re-hash the contents and resolve the error
-
-`)
+func printChecksumError(cmd *cobra.Command, err error) {
 	cmd.SilenceUsage = true
+	out := cmd.OutOrStderr()
+	fmt.Fprintln(out, "You have a checksum error in your migration directory.")
+	if csErr := (&migrate.ChecksumError{}); errors.As(err, &csErr) {
+		fmt.Fprintf(out, "\n\tL %d: %s was %s\n\n", csErr.Line, csErr.File, csErr.Reason)
+	}
+	fmt.Fprintf(
+		out,
+		"Please check your migration files and run %v to re-hash the contents\n\n",
+		cmdlog.ColorCyan("'atlas migrate hash'"),
+	)
 }
 
 // selectScheme validates the scheme of the provided to urls and returns the selected
diff --git a/cmd/atlas/internal/migratelint/lint.go b/cmd/atlas/internal/migratelint/lint.go
index 34315eba..54df932c 100644
--- a/cmd/atlas/internal/migratelint/lint.go
+++ b/cmd/atlas/internal/migratelint/lint.go
@@ -363,3 +363,5 @@ type FileError struct {
 }
 
 func (e FileError) Error() string { return e.Err.Error() }
+
+func (e FileError) Unwrap() error { return e.Err }
diff --git a/cmd/atlas/internal/migratelint/run.go b/cmd/atlas/internal/migratelint/run.go
index fdd11449..89e39bb2 100644
--- a/cmd/atlas/internal/migratelint/run.go
+++ b/cmd/atlas/internal/migratelint/run.go
@@ -88,8 +88,18 @@ func (r *Runner) summary(ctx context.Context) error {
 	switch err := migrate.Validate(r.Dir); {
 	case errors.Is(err, migrate.ErrChecksumNotFound):
 	case err != nil:
-		err := &FileError{File: migrate.HashFileName, Err: err}
-		r.sum.Files = append(r.sum.Files, &FileReport{Name: migrate.HashFileName, Error: err.Error()})
+		var (
+			err = &FileError{File: migrate.HashFileName, Err: err}
+			rep = &FileReport{Name: migrate.HashFileName, Error: err.Error()}
+		)
+		if csErr := (&migrate.ChecksumError{}); errors.As(err, &csErr) {
+			err.Pos = csErr.Pos
+			rep = &FileReport{
+				Name:  migrate.HashFileName,
+				Error: fmt.Sprintf("%s (atlas.sum): L%d: %s was %s", csErr, csErr.Line, csErr.File, csErr.Reason),
+			}
+		}
+		r.sum.Files = append(r.sum.Files, rep)
 		return r.sum.StepError(StepIntegrityCheck, fmt.Sprintf("File %s is invalid", migrate.HashFileName), err)
 	default:
 		// If the hash file exists, it is valid.
@@ -510,6 +520,8 @@ func (w *TemplateWriter) WriteReport(r *SummaryReport) error {
 	return w.T.Execute(w.W, r)
 }
 
+func (err SilentError) Unwrap() error { return err.error }
+
 func nolintRules(f *sqlcheck.File) *skipRules {
 	s := &skipRules{pos2rules: make(map[int][]string)}
 	if l, ok := f.File.(*migrate.LocalFile); ok {
diff --git a/doc/md/guides/guides.mdx b/doc/md/guides/guides.mdx
index d7ca3105..2bb4c16c 100644
--- a/doc/md/guides/guides.mdx
+++ b/doc/md/guides/guides.mdx
@@ -165,9 +165,9 @@ import { Card, CardImage } from "../components/card";
     image={CardImage.Postgres}
   />
   <Card
-    name="Optimal data alignment (AR101)"
+    name="Optimal data alignment (PG110)"
     description="This guide is for achieving optimal data alignment in PostgreSQL with Atlas"
-    url="/guides/postgres/ar-101"
+    url="/guides/postgres/pg-110"
     image={CardImage.Postgres}
   />
   <Card
diff --git a/doc/md/guides/postgres/ar-101.md b/doc/md/guides/postgres/pg-110.md
similarity index 97%
rename from doc/md/guides/postgres/ar-101.md
rename to doc/md/guides/postgres/pg-110.md
index 12c774eb..520ef9da 100644
--- a/doc/md/guides/postgres/ar-101.md
+++ b/doc/md/guides/postgres/pg-110.md
@@ -1,7 +1,7 @@
 ---
-id: ar-101
-title: Optimal data alignment in tables (AR101)
-slug: /guides/postgres/ar-101
+id: pg-110
+title: Optimal data alignment in tables (PG110)
+slug: /guides/postgres/pg-110
 ---
 
 ### Alignment and Padding in PostgreSQL
@@ -183,9 +183,9 @@ CREATE TABLE users (
 );
 ```
 
-Once the Atlas CI check is completed, you should see the following message inside Atlas Lint Report:
+Once the Atlas CI check is completed, you should see the following code suggestion in your pull request:
 
-![atlasci](https://atlasgo.io/uploads/images/atlas-lint-report.png)
+![atlasci](https://atlasgo.io/uploads/images/atlas-lint-suggestion.png)
 
 Awesome! The Atlas CI check has analyzed the migration file and suggested the best possible arrangement of columns in order to optimize the space taken by padding per row.
 
diff --git a/doc/md/lint/analyzers.mdx b/doc/md/lint/analyzers.mdx
index 78e335e8..9492a19a 100644
--- a/doc/md/lint/analyzers.mdx
+++ b/doc/md/lint/analyzers.mdx
@@ -232,8 +232,6 @@ The following schema change checks are provided by Atlas:
 
 | **Check**                                 | **Short Description**                                                                        |
 |-------------------------------------------|----------------------------------------------------------------------------------------------|
-| **AR**                                    | Atlas cloud checks                                                                           |
-| [AR101](#AR101)                           | Creating table with non-optimal data alignment                                               |
 | **BC**                                    | **[Backward incompatible changes](#backward-incompatible-changes)**                          |
 | [BC101](#BC101)                           | Renaming a table                                                                             |
 | [BC102](#BC102)                           | Renaming a column                                                                            |
@@ -268,6 +266,8 @@ The following schema change checks are provided by Atlas:
 | [NM104](#NM104)                           | Index name violates the naming convention                                                    |
 | [NM105](#NM105)                           | Foreign-key constraint name violates the naming convention                                   |
 | [NM106](#NM106)                           | Check constraint name violates the naming convention                                         |
+| **PG**                                    | PostgreSQL specific checks                                                                   |
+| [PG110](#PG110)                           | Creating table with non-optimal data alignment                                               |
 | **PG1**                                   | **[Concurrent Indexes](#concurrent-index-policy-postgresql)**                                |
 | [PG101](#PG101)                           | Missing the `CONCURRENTLY` in index creation                                                 |
 | [PG102](#PG102)                           | Missing the `CONCURRENTLY` in index deletion                                                 |
@@ -556,7 +556,7 @@ ALTER TABLE `new_users` RENAME TO `users`;
 PRAGMA foreign_keys = on;
 ```
 
-#### AR101 {#AR101}
+#### PG110 {#PG110}
 
 Creating a table with optimal data alignment may help minimize the amount of required disk space.
 For example consider the next Postgres table on a 64-bit system:
diff --git a/doc/website/sidebars.js b/doc/website/sidebars.js
index b40127bb..ebbd75ab 100644
--- a/doc/website/sidebars.js
+++ b/doc/website/sidebars.js
@@ -438,8 +438,8 @@ module.exports = {
                 },
                 {
                     type: 'doc',
-                    id: 'guides/postgres/ar-101',
-                    label: 'Optimal data alignment (AR101)'
+                    id: 'guides/postgres/pg-110',
+                    label: 'Optimal data alignment (PG110)'
                 },
                 {
                     type: 'doc',
diff --git a/sql/migrate/dir.go b/sql/migrate/dir.go
index 6971e1e5..74b30ebb 100644
--- a/sql/migrate/dir.go
+++ b/sql/migrate/dir.go
@@ -17,6 +17,7 @@ import (
 	"os"
 	"path/filepath"
 	"regexp"
+	"slices"
 	"sort"
 	"strings"
 	"sync"
@@ -625,21 +626,60 @@ func (f HashFile) SumByName(n string) (string, error) {
 	return "", errors.New("checksum not found")
 }
 
+// Reason for a checksum mismatch.
+const (
+	ReasonAdded Reason = iota + 1
+	ReasonEdited
+	ReasonRemoved
+)
+
 var (
 	// ErrChecksumFormat is returned from Validate if the sum files format is invalid.
 	ErrChecksumFormat = errors.New("checksum file format invalid")
-	// ErrChecksumMismatch is returned from Validate if the hash sums don't match.
+	// ErrChecksumMismatch is returned when unmarshalling from a sum file and the files sum entries don't match.
 	ErrChecksumMismatch = errors.New("checksum mismatch")
 	// ErrChecksumNotFound is returned from Validate if the hash file does not exist.
 	ErrChecksumNotFound = errors.New("checksum file not found")
 )
 
+type (
+	// ChecksumError indicates a mismatch between a directories files and its sum.
+	ChecksumError struct {
+		Line, Total, Pos int    // line number in file of the mismatch, total number of lines, pos in file
+		File             string // filename of the mismatch
+		Reason           Reason // reason of a mismatch by filename
+	}
+	// Reason for a checksum mismatch.
+	Reason uint
+)
+
+// Error implements the error interface.
+func (err ChecksumError) Error() string { return ErrChecksumMismatch.Error() }
+
+// Is exists for backwards compatability reasons.
+func (err ChecksumError) Is(target error) bool {
+	return errors.Is(ErrChecksumMismatch, target)
+}
+
+// String implements fmt.Stringer.
+func (r Reason) String() string {
+	switch r {
+	case ReasonAdded:
+		return "added"
+	case ReasonEdited:
+		return "edited"
+	case ReasonRemoved:
+		return "removed"
+	}
+	return "unknown reason"
+}
+
 // Validate checks if the migration dir is in sync with its sum file.
 // If they don't match ErrChecksumMismatch is returned.
 func Validate(dir Dir) error {
-	fh, err := readHashFile(dir)
+	ac, err := readHashFile(dir)
 	if errors.Is(err, fs.ErrNotExist) {
-		// If there are no migration files yet this is okay.
+		// If there are no migration files yet, this is okay.
 		if files, err := dir.Files(); err != nil {
 			return err
 		} else if len(files) > 0 {
@@ -650,12 +690,46 @@ func Validate(dir Dir) error {
 	if err != nil {
 		return err
 	}
-	mh, err := dir.Checksum()
+	ex, err := dir.Checksum()
 	if err != nil {
 		return err
 	}
-	if fh.Sum() != mh.Sum() {
-		return ErrChecksumMismatch
+	if ac.Sum() != ex.Sum() {
+		err := &ChecksumError{Total: len(ac)}
+		// Determine the reason for the mismatch. Iterate over the file sum,
+		// based on it determine if a file was removed, added or edited.
+		const hashSize = 3 + 44 // h1: (3) + base64(sha256sum) (44)
+		pos := hashSize + 1     // total hash + newline
+		for i, h := range ac {
+			// Proceed until we find the mismatch.
+			if len(ex) > i && ex[i] == h {
+				pos += len(h.N) + 1 + hashSize + 1 // filename + space + hash + newline
+				continue
+			}
+			// Index is now pointing at the file with the mismatch.
+			err.Line = i + 1
+			err.Pos = pos
+			err.File = h.N
+			switch idx := slices.IndexFunc(ex, func(e struct{ N, H string }) bool { return e.N == h.N }); {
+			case idx < 0:
+				err.Reason = ReasonRemoved
+			case idx == i:
+				// If the file is in its original place, it was edited.
+				err.Reason = ReasonEdited
+			default:
+				// File was not in its original place, meaning another file was added before it.
+				err.File = ex[i].N
+				err.Reason = ReasonAdded
+			}
+			return err
+		}
+		// If we land here, all migrations in the sum file are present unchanged in the computed sum.
+		// But there is a mismatch, meaning the next file in the computed sum was added.
+		err.Line = err.Total + 1
+		err.File = ex[err.Total].N
+		err.Pos = pos
+		err.Reason = ReasonAdded
+		return err
 	}
 	return nil
 }


Test patch:
diff --git a/cmd/atlas/internal/cmdapi/migrate_test.go b/cmd/atlas/internal/cmdapi/migrate_test.go
index 9fdd38c5..cd8f64a8 100644
--- a/cmd/atlas/internal/cmdapi/migrate_test.go
+++ b/cmd/atlas/internal/cmdapi/migrate_test.go
@@ -1878,6 +1878,24 @@ func TestMigrate_Validate(t *testing.T) {
 	// Should fail since the files are not compatible with SQLite.
 	_, err = runCmd(migrateValidateCmd(), "--dir", "file://testdata/mysql", "--dev-url", openSQLite(t, ""))
 	require.Error(t, err)
+
+	// Will report detailed information when there is a checksum mismatch.
+	p = t.TempDir()
+	require.NoError(t, os.WriteFile(filepath.Join(p, "1_initial.sql"), []byte("create table t1 (c1 int)"), 0644))
+	s, err = runCmd(migrateValidateCmd(), "--dir", "file://"+p)
+	require.ErrorIs(t, err, migrate.ErrChecksumNotFound)
+	require.Contains(t, s, "You have a checksum error")
+	_, err = runCmd(migrateHashCmd(), "--dir", "file://"+p)
+	require.NoError(t, err)
+	require.NoError(t, os.WriteFile(filepath.Join(p, "2_second.sql"), []byte("create table t2 (c2 int)"), 0644))
+	s, err = runCmd(migrateValidateCmd(), "--dir", "file://"+p)
+	csErr := &migrate.ChecksumError{}
+	require.ErrorAs(t, err, &csErr)
+	require.Equal(t, 2, csErr.Line)
+	require.Equal(t, "2_second.sql", csErr.File)
+	require.Equal(t, migrate.ReasonAdded, csErr.Reason)
+	require.Contains(t, s, "You have a checksum error")
+	require.Contains(t, s, "L 2: 2_second.sql was added")
 }
 
 func TestMigrate_Hash(t *testing.T) {
diff --git a/cmd/atlas/internal/migratelint/run_test.go b/cmd/atlas/internal/migratelint/run_test.go
index d4a72f0a..98bfd52a 100644
--- a/cmd/atlas/internal/migratelint/run_test.go
+++ b/cmd/atlas/internal/migratelint/run_test.go
@@ -24,6 +24,76 @@ func TestRunner_Run(t *testing.T) {
 	b := &bytes.Buffer{}
 	c, err := sqlclient.Open(ctx, "sqlite://run?mode=memory&cache=shared&_fk=1")
 	require.NoError(t, err)
+
+	t.Run("checksum mismatch", func(t *testing.T) {
+		var (
+			d = &migrate.MemDir{}
+			r = &migratelint.Runner{
+				Dir: d,
+				Dev: c,
+				ReportWriter: &migratelint.TemplateWriter{
+					T: migratelint.DefaultTemplate,
+					W: b,
+				},
+			}
+			err = &migrate.ChecksumError{}
+		)
+		// File was added at the end.
+		require.NoError(t, migrate.WriteSumFile(d, migrate.HashFile{}))
+		require.NoError(t, d.WriteFile("1.sql", []byte("content")))
+		require.ErrorAs(t, r.Run(ctx), &err)
+		require.Regexp(t, `Analyzing changes \(1 migration in total\):
+
+  Error: checksum mismatch \(atlas.sum\): L1: 1\.sql was added
+
+  -------------------------
+  -- .*
+  -- 1 version with errors
+`, b.String())
+		// File was edited.
+		b.Reset()
+		require.NoError(t, migrate.WriteSumFile(d, must(d.Checksum())))
+		require.NoError(t, d.WriteFile("1.sql", []byte("content changed")))
+		require.ErrorAs(t, r.Run(ctx), &err)
+		require.Regexp(t, `Analyzing changes \(1 migration in total\):
+
+  Error: checksum mismatch \(atlas.sum\): L1: 1\.sql was edited
+
+  -------------------------
+  -- .*
+  -- 1 version with errors
+`, b.String())
+		// File was removed.
+		b.Reset()
+		h := must(d.Checksum())
+		*d = migrate.MemDir{}
+		require.NoError(t, migrate.WriteSumFile(d, h))
+		require.ErrorAs(t, r.Run(ctx), &err)
+		require.Regexp(t, `Analyzing changes \(1 migration in total\):
+
+  Error: checksum mismatch \(atlas.sum\): L1: 1\.sql was removed
+
+  -------------------------
+  -- .*
+  -- 1 version with errors
+`, b.String())
+		// File was added in the middle.
+		b.Reset()
+		require.NoError(t, d.WriteFile("1.sql", []byte("content")))
+		require.NoError(t, d.WriteFile("3.sql", []byte("content")))
+		require.NoError(t, migrate.WriteSumFile(d, must(d.Checksum())))
+		require.NoError(t, d.WriteFile("2.sql", []byte("content")))
+		require.ErrorAs(t, r.Run(ctx), &err)
+		require.Regexp(t, `Analyzing changes \(1 migration in total\):
+
+  Error: checksum mismatch \(atlas.sum\): L2: 2\.sql was added
+
+  -------------------------
+  -- .*
+  -- 1 version with errors
+`, b.String())
+	})
+
 	az := &testAnalyzer{
 		reports: []sqlcheck.Report{
 			{Text: "Report 1", Diagnostics: []sqlcheck.Diagnostic{{Pos: 1, Text: "Diagnostic 1", Code: "TS101"}}},
@@ -203,3 +273,10 @@ type testDetector struct {
 func (t testDetector) DetectChanges(context.Context) ([]migrate.File, []migrate.File, error) {
 	return t.base, t.feat, nil
 }
+
+func must[T any](t T, err error) T {
+	if err != nil {
+		panic(err)
+	}
+	return t
+}
diff --git a/sql/migrate/dir_test.go b/sql/migrate/dir_test.go
index 7559dbd9..4f5098d7 100644
--- a/sql/migrate/dir_test.go
+++ b/sql/migrate/dir_test.go
@@ -78,8 +78,12 @@ func TestHashSum(t *testing.T) {
 	require.NotContains(t, string(c), "exclude_2.sql")
 }
 
-//go:embed testdata/migrate/atlas.sum
-var hash []byte
+var (
+	//go:embed testdata/migrate/atlas.sum
+	hash []byte
+	//go:embed testdata/migrate/1_initial.up.sql
+	initialUp []byte
+)
 
 func TestValidate(t *testing.T) {
 	// Add the sum file form the testdata/migrate dir without any files in it - should fail.
@@ -87,7 +91,7 @@ func TestValidate(t *testing.T) {
 	d, err := migrate.NewLocalDir(p)
 	require.NoError(t, err)
 	require.NoError(t, d.WriteFile("atlas.sum", hash))
-	require.Equal(t, migrate.ErrChecksumMismatch, migrate.Validate(d))
+	require.Equal(t, removed(1, 2, 48, "1_initial.down.sql"), migrate.Validate(d))
 
 	td := "testdata/migrate"
 	d, err = migrate.NewLocalDir(td)
@@ -115,19 +119,37 @@ func TestValidate(t *testing.T) {
 	require.Equal(t, migrate.ErrChecksumFormat, migrate.Validate(d))
 	require.NoError(t, os.WriteFile(filepath.Join(td, "atlas.sum"), hash, 0644))
 
+	// Changing the contents of the file will report it edited.
+	f, err = os.OpenFile(filepath.Join(td, "1_initial.up.sql"), os.O_RDWR, os.ModeAppend)
+	require.NoError(t, err)
+	_, err = f.WriteString("something")
+	require.NoError(t, err)
+	require.NoError(t, f.Close())
+	t.Cleanup(func() {
+		require.NoError(t, os.WriteFile(filepath.Join(td, "1_initial.up.sql"), initialUp, 0644))
+	})
+	require.Equal(t, edited(2, 2, 115, "1_initial.up.sql"), migrate.Validate(d))
+	require.NoError(t, os.WriteFile(filepath.Join(td, "1_initial.up.sql"), initialUp, 0644))
+
+	// Adding a file at the end.
+	require.NoError(t, os.WriteFile(filepath.Join(td, "2_second.sql"), []byte("stmt"), os.ModePerm))
+	t.Cleanup(func() { os.Remove(filepath.Join(td, "2_second.sql")) })
+	require.Equal(t, added(3, 2, 180, "2_second.sql"), migrate.Validate(d))
+	require.NoError(t, os.Remove(filepath.Join(td, "2_second.sql")))
+
 	// Changing the filename should raise validation error.
 	require.NoError(t, os.Rename(filepath.Join(td, "1_initial.up.sql"), filepath.Join(td, "1_first.up.sql")))
 	t.Cleanup(func() {
 		require.NoError(t, os.Rename(filepath.Join(td, "1_first.up.sql"), filepath.Join(td, "1_initial.up.sql")))
 	})
-	require.Equal(t, migrate.ErrChecksumMismatch, migrate.Validate(d))
+	require.Equal(t, added(1, 2, 48, "1_first.up.sql"), migrate.Validate(d))
 
 	// Removing it as well (move it out of the dir).
 	require.NoError(t, os.Rename(filepath.Join(td, "1_first.up.sql"), filepath.Join(td, "..", "bak")))
 	t.Cleanup(func() {
 		require.NoError(t, os.Rename(filepath.Join(td, "..", "bak"), filepath.Join(td, "1_first.up.sql")))
 	})
-	require.Equal(t, migrate.ErrChecksumMismatch, migrate.Validate(d))
+	require.Equal(t, removed(2, 2, 115, "1_initial.up.sql"), migrate.Validate(d))
 }
 
 func TestHash_MarshalText(t *testing.T) {
@@ -507,3 +529,25 @@ func fileNames(r io.Reader) ([]string, error) {
 	}
 	return out, nil
 }
+
+func removed(line, total, pos int, file string) *migrate.ChecksumError {
+	return reason(line, total, pos, file, migrate.ReasonRemoved)
+}
+
+func added(line, total, pos int, file string) *migrate.ChecksumError {
+	return reason(line, total, pos, file, migrate.ReasonAdded)
+}
+
+func edited(line, total, pos int, file string) *migrate.ChecksumError {
+	return reason(line, total, pos, file, migrate.ReasonEdited)
+}
+
+func reason(line, total, pos int, file string, reason migrate.Reason) *migrate.ChecksumError {
+	return &migrate.ChecksumError{
+		Line:   line,
+		Pos:    pos,
+		Total:  total,
+		File:   file,
+		Reason: reason,
+	}
+}