Add line/column/position details to Postgres migration/rollback query…

… error messages (#495) Postgres's database driver returns the character position where a syntax error occurs, so dbmate can use that information to calculate the line and column within the query that the position refers to. Co-authored-by: Jae B <[email protected]> Co-authored-by: Adrian Macneil <[email protected]>
amacneil · Nov 16, 2023 · d74b06f · d74b06f
1 parent 1b5ca4f
commit d74b06f
Show file tree

Hide file tree

Showing 7 changed files with 142 additions and 2 deletions.
diff --git a/pkg/dbmate/db.go b/pkg/dbmate/db.go
@@ -346,7 +346,7 @@ func (db *DB) Migrate() error {
 			// run actual migration
 			result, err := tx.Exec(parsed.Up)
 			if err != nil {
-				return err
+				return drv.QueryError(parsed.Up, err)
 			} else if db.Verbose {
 				db.printVerbose(result)
 			}
@@ -508,7 +508,7 @@ func (db *DB) Rollback() error {
 		// rollback migration
 		result, err := tx.Exec(parsed.Down)
 		if err != nil {
-			return err
+			return drv.QueryError(parsed.Down, err)
 		} else if db.Verbose {
 			db.printVerbose(result)
 		}

diff --git a/pkg/dbmate/db_test.go b/pkg/dbmate/db_test.go
@@ -659,3 +659,79 @@ func TestMigrateStrictOrder(t *testing.T) {
 	err = db.Migrate()
 	require.Error(t, err)
 }
+
+func TestMigrateQueryErrorMessage(t *testing.T) {
+	u := dbutil.MustParseURL(os.Getenv("POSTGRES_TEST_URL"))
+	db := newTestDB(t, u)
+
+	err := db.Drop()
+	require.NoError(t, err)
+	err = db.Create()
+	require.NoError(t, err)
+
+	t.Run("ASCII SQL, error in migrate up", func(t *testing.T) {
+		db.FS = fstest.MapFS{
+			"db/migrations/001_ascii_error_up.sql": {
+				Data: []byte("-- migrate:up\n-- line 2\nnot_valid_sql;\n-- migrate:down"),
+			},
+		}
+
+		err = db.Migrate()
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "line: 3, column: 1, position: 25:")
+	})
+
+	t.Run("ASCII SQL, error in migrate down", func(t *testing.T) {
+		db.FS = fstest.MapFS{
+			"db/migrations/002_ascii_error_down.sql": {
+				Data: []byte("-- migrate:up\n--migrate:down\n  not_valid_sql; -- indented"),
+			},
+		}
+
+		err = db.Migrate()
+		require.NoError(t, err)
+
+		err = db.Rollback()
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "line: 2, column: 3, position: 18:")
+	})
+
+	t.Run("UTF-8 SQL, error in migrate up", func(t *testing.T) {
+		db.FS = fstest.MapFS{
+			"db/migrations/003_utf8_error_up.sql": {
+				Data: []byte("-- migrate:up\n-- line 2\n/* สวัสดี hello */ not_valid_sql;\n--migrate:down"),
+			},
+		}
+
+		err = db.Migrate()
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "line: 3, column: 20, position: 44:")
+	})
+
+	t.Run("UTF-8 SQL, error in migrate down", func(t *testing.T) {
+		db.FS = fstest.MapFS{
+			"db/migrations/004_utf8_error_up.sql": {
+				Data: []byte("-- migrate:up\n-- migrate:down\n/* สวัสดี hello */ not_valid_sql;"),
+			},
+		}
+
+		err = db.Migrate()
+		require.NoError(t, err)
+
+		err = db.Rollback()
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "line: 2, column: 20, position: 36:")
+	})
+
+	t.Run("correctly count with CR-LF line endings present", func(t *testing.T) {
+		db.FS = fstest.MapFS{
+			"db/migrations/005_cr_lf_line_endings.sql": {
+				Data: []byte("-- migrate:up\r\n-- line 2\r\n  not_valid_sql; -- indented\r\n-- migrate:down"),
+			},
+		}
+
+		err = db.Migrate()
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "line: 3, column: 3, position: 29:")
+	})
+}
diff --git a/pkg/dbmate/driver.go b/pkg/dbmate/driver.go
@@ -2,6 +2,7 @@ package dbmate
 
 import (
 	"database/sql"
+	"fmt"
 	"io"
 	"net/url"
 
@@ -21,6 +22,7 @@ type Driver interface {
 	InsertMigration(dbutil.Transaction, string) error
 	DeleteMigration(dbutil.Transaction, string) error
 	Ping() error
+	QueryError(string, error) error
 }
 
 // DriverConfig holds configuration passed to driver constructors
@@ -33,6 +35,39 @@ type DriverConfig struct {
 // DriverFunc represents a driver constructor
 type DriverFunc func(DriverConfig) Driver
 
+type QueryError struct {
+	Err      error
+	Query    string
+	Position int
+}
+
+func (e *QueryError) Error() string {
+	if e.Position > 0 {
+		line := 1
+		column := 1
+		offset := 0
+		for _, ch := range e.Query {
+			offset++
+			if offset >= e.Position {
+				break
+			}
+			// don't count CR as a column in CR/LF sequences
+			if ch == '\r' {
+				continue
+			}
+			if ch == '\n' {
+				line++
+				column = 1
+				continue
+			}
+			column++
+		}
+		return fmt.Sprintf("line: %d, column: %d, position: %d: %s", line, column, e.Position, e.Err.Error())
+	}
+
+	return e.Err.Error()
+}
+
 var drivers = map[string]DriverFunc{}
 
 // RegisterDriver registers a driver constructor for a given URL scheme

diff --git a/pkg/driver/clickhouse/clickhouse.go b/pkg/driver/clickhouse/clickhouse.go
@@ -369,6 +369,11 @@ func (drv *Driver) Ping() error {
 	return err
 }
 
+// Return a normalized version of the driver-specific error type.
+func (drv *Driver) QueryError(query string, err error) error {
+	return &dbmate.QueryError{Err: err, Query: query}
+}
+
 func (drv *Driver) quotedMigrationsTableName() string {
 	return drv.quoteIdentifier(drv.migrationsTableName)
 }
diff --git a/pkg/driver/mysql/mysql.go b/pkg/driver/mysql/mysql.go
@@ -313,6 +313,11 @@ func (drv *Driver) Ping() error {
 	return db.Ping()
 }
 
+// Return a normalized version of the driver-specific error type.
+func (drv *Driver) QueryError(query string, err error) error {
+	return &dbmate.QueryError{Err: err, Query: query}
+}
+
 func (drv *Driver) quotedMigrationsTableName() string {
 	return drv.quoteIdentifier(drv.migrationsTableName)
 }
diff --git a/pkg/driver/postgres/postgres.go b/pkg/driver/postgres/postgres.go
@@ -7,6 +7,7 @@ import (
 	"io"
 	"net/url"
 	"runtime"
+	"strconv"
 	"strings"
 
 	"github.com/amacneil/dbmate/v2/pkg/dbmate"
@@ -363,6 +364,19 @@ func (drv *Driver) Ping() error {
 	return err
 }
 
+// Return a normalized version of the driver-specific error type.
+func (drv *Driver) QueryError(query string, err error) error {
+	position := 0
+
+	if pqErr, ok := err.(*pq.Error); ok {
+		if pos, err := strconv.Atoi(pqErr.Position); err == nil {
+			position = pos
+		}
+	}
+
+	return &dbmate.QueryError{Err: err, Query: query, Position: position}
+}
+
 func (drv *Driver) quotedMigrationsTableName(db dbutil.Transaction) (string, error) {
 	schema, name, err := drv.quotedMigrationsTableNameParts(db)
 	if err != nil {

diff --git a/pkg/driver/sqlite/sqlite.go b/pkg/driver/sqlite/sqlite.go
@@ -243,6 +243,11 @@ func (drv *Driver) Ping() error {
 	return db.Ping()
 }
 
+// Return a normalized version of the driver-specific error type.
+func (drv *Driver) QueryError(query string, err error) error {
+	return &dbmate.QueryError{Err: err, Query: query}
+}
+
 func (drv *Driver) quotedMigrationsTableName() string {
 	return drv.quoteIdentifier(drv.migrationsTableName)
 }