:wave: hey there I came across this package (which looks really cool) and I noticed this line:

if thisVersion > currentVersion {
	continue
}

I was wondering how the version compare would handle this scenario:

9.up.sql
10.up.sql

given the following result when comparing the two version strings:

fmt.Println("9" > "10")
// true

have you considered parsing the version as a number?

or maybe you would consider allowing the user to customize the regular expression to ensure extra safety? For example I could customize the up and down expressions to ensure the version is expressed as a date:

// 20210510.up.sql
regexp.MustCompile(`^([\d]{7}).up.sql$`)

or I could customize the expression to ignore the trailing description and only use the digits for comparison:

// 20210510-create-user-table.up.sql
regexp.MustCompile(`^([\d]{7})[\w-]+.up.sql$`)

I think it would be useful and more flexible. Maybe with some functional options or just another New* function.

• Just an exported field:

type Migrator struct {
	DB *sql.DB
	FS fs.FS
	Table string
}

m := New(db, migrations)
m.Table = "migrate_inator"

• With another New* function:

type Migrator struct {
	DB *sql.DB
	FS fs.FS
	table string
}

// New Migrator with default options.
func New(db *sql.DB, fs fs.FS) *Migrator {
	return &Migrator{
		DB: db,
		FS: fs,
		table: "migrate",
	}
}

func NewWithTable(db *sql.DB, fs fs.FS, table string) *Migrator {
	return &Migrator{
		DB: db,
		FS: fs,
		table: table,
	}
}

m := NewWithTable(db, migrations, "migrate_inator")

• With a Config struct:

type Config struct {
	DB *sql.DB
	FS fs.FS
	Table string
}

type Migrator struct {
	DB *sql.DB
	FS fs.FS
	table string
}

// New Migrator with default options.
func New(db *sql.DB, fs fs.FS) *Migrator {
	return NewWithConfig(Config{
		DB: db,
		FS: fs,
		Table: "migrate",
	})
}

func NewWithConfig(cfg Config) *Migrator {
	return &Migrator{
		DB: cfg.DB,
		FS: cfg.FS,
		table: cfg.Table,
	}
}

m := NewWithConfig(Config{
	DB: db,
	FS: migrations,
	Table: "migrate_inator"
})

• With functional options:

type Migrator struct {
	DB *sql.DB
	FS fs.FS
	table string
}

type Option func(*Migrator)

// New Migrator with default options.
func New(db *sql.DB, fs fs.FS, options ...Option) *Migrator {
	m := &Migrator{
		DB: db,
		FS: fs,
		table: "migrate",
	}

	for _, opt := range options {
		opt(m)
	}

	return m
}

func WithTable(table string) Option {
	return func(m *Migrator) {
		m.table = table
	}
}

m := New(db, migrations, WithTable("migrate_inator"))

Currently it is impossible to create a migration table with name master.migrations for example. The reason is the regexp that check the name https://github.com/maragudk/migrate/blob/507b57a50cdcbdaacc8cb3d0a4b0c5fb67187d1a/migrate.go#L18

This can be useful when a Postgresql DB contains multiple schemas and you want to create migration table in a specific one.

I suggest to add '.' as a valid character tableMatcher = regexp.MustCompile(^[\w\.]+$)

👋 hey there, I have an interesting use case that is not solved by most migrations tools, and I was wondering if you would be open to the suggestion. First please allow me to describe the problem.

I have a database that supports postgres and sqlite, however, the ddl syntax can be slightly different which means I need two different (but very similar) sets of migrations. In the interest of reducing a significant amount of duplicate code, it would be great if I could leverage templates to have a single set of migrations.

For example:

CREATE TABLE IF NOT EXISTS person (
 id      {{ if eq .driver "sqlite3" }}INTEGER PRIMARY KEY AUTOINCREMENT{{ else }}SERIAL PRIMARY KEY{{ end }}
,name    TEXT
,last    TEXT
,first   TEXT
,address TEXT
,dob     DATE
);

I suspect that templating would be outside the scope of this library, for good reason. Instead, I would propose that this library provides a hook for pre-processing. This would allow me to provide a callback function that executes a template and returns the final sql statement. For example:

		// Normally we wouldn't just string interpolate the version like this,
		// but because we know the version has been matched against the regexes, we know it's safe.
		if _, err := tx.ExecContext(ctx, `update `+m.table+` set version = '`+version+`'`); err != nil {
			return err
		}

+		stmt := string(content)
+		if m.process != nil {
+			stmt = m.process(ctx, stmt)
+		}
+		if _, err := tx.ExecContext(ctx, stmt); err != nil {
-		if _, err := tx.ExecContext(ctx, string(content)); err != nil {
			return err
		}

This would allow me to do something like this:

driver := "postgres"
opts.Process = func(before string) (string, error) {
	t, err := template.New("_").Parse(before)
	if err != nil {
		return before, err
	}
	var buf bytes.Buffer
	err := t.Execute(buf, map[string]string{"driver": driver})
	return buf.String(), err
}

Alternatively, instead of a pre-processing hook, you could allow the user to override the execution:

+		if m.exec != nil {
+			if err := m.exec(ctx, tx, string(content)); err != nil {
+				return err
+			}
+		} else {
			if _, err := tx.ExecContext(ctx, string(content)); err != nil {
				return err
			}
+		}

I probably prefer the first option but the second option might provide some additional flexibility (for example, maybe I want to log the sql statement on error, or skip execution if the template generates an empty string). I think this would be a pretty small change that would solve a very real world problem for authors that are support multiple database vendors. If you are open to supporting this feature, I would be happy to submit a pull request.

https://github.com/maragudk/migrate/blob/b86d7a9b2a9cf1e643280287337756db6bdb96af/migrate.go#L293

In some scenario a public usage of this function can be interesting, for example for a migration tool based on your library.

I can propose a PR about this, if the behavior is considered useful.

This adds optional functions in the migrate.Options that, if set, are called before and after each migration. The functions are passed the context, the transaction, and the version string. If the callback errors, the current migration is aborted and the transaction rolled back.

Because the returned error from Migrator.inTransaction wasn't named, the recover() from within the deferred function didn't actually have an effect setting the err variable.