maragudk / migrate Goto Github PK

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.

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"))

👋 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$`)

Per flavor.

Currently it is impossible to create a migration table with name master.migrations for example.
The reason is the regexp that check the name

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.

I've read that mysql/mariadb can leave the db in a dirty state even when running migrations in a transaction. Check whether this is true.