Skip to content

Do not create migrations table on dump#701

Open
MatthiasKunnen wants to merge 1 commit into
amacneil:mainfrom
MatthiasKunnen:dump-no-changes
Open

Do not create migrations table on dump#701
MatthiasKunnen wants to merge 1 commit into
amacneil:mainfrom
MatthiasKunnen:dump-no-changes

Conversation

@MatthiasKunnen
Copy link
Copy Markdown

@MatthiasKunnen MatthiasKunnen commented Nov 13, 2025

When running dbmate dump, no changes to the database should be made.

Use case: running dbmate dump with credentials that do not have CREATE permissions.

@MatthiasKunnen MatthiasKunnen changed the title Do not create migrations table on dump wip: Do not create migrations table on dump Nov 13, 2025
@MatthiasKunnen MatthiasKunnen marked this pull request as draft November 13, 2025 00:59
@MatthiasKunnen MatthiasKunnen changed the title wip: Do not create migrations table on dump Do not create migrations table on dump Nov 13, 2025
@MatthiasKunnen MatthiasKunnen marked this pull request as ready for review November 15, 2025 21:43
When running dbmate dump, no changes to the database should be made.
Comment thread pkg/dbmate/db_test.go
err := db.Drop()
require.NoError(t, err)

// create and migrate
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: create and don't migrate

func (drv *Driver) schemaMigrationsDump(db *sql.DB) ([]byte, error) {
exists, err := drv.MigrationsTableExists(db)
if err != nil {
return nil, fmt.Errorf("failed to check if migration table exists: %w", err)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't appear to be a failure to check (we just tried to), but that we were unable to verify the existence of the migrations table? Saying "failed to check" seems a bit misleading here.

Would return nil, err be sufficient here, assuming that the error being returned from MigrationsTableExists() is well-formed?

func (drv *Driver) schemaMigrationsDump(db *sql.DB, buf *bytes.Buffer) error {
exists, err := drv.MigrationsTableExists(db)
if err != nil {
return fmt.Errorf("failed to check if migration table exists: %w", err)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See previous comment about returning err as-is.

Comment thread pkg/driver/mysql/mysql.go
func (drv *Driver) schemaMigrationsDump(db *sql.DB) ([]byte, error) {
exists, err := drv.MigrationsTableExists(db)
if err != nil {
return nil, fmt.Errorf("failed to check if migration table exists: %w", err)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See previous comment about returning err as-is.

func (drv *Driver) schemaMigrationsDump(db *sql.DB) ([]byte, error) {
exists, err := drv.MigrationsTableExists(db)
if err != nil {
return nil, fmt.Errorf("failed to check if migration table exists: %w", err)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See previous comment about returning err as-is.

func (drv *Driver) schemaMigrationsDump(db *sql.DB) ([]byte, error) {
exists, err := drv.MigrationsTableExists(db)
if err != nil {
return nil, fmt.Errorf("failed to check if migration table exists: %w", err)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See previous comment about returning err as-is.

@dossy dossy added go Pull requests that update Go code needs work labels Feb 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

go Pull requests that update Go code needs work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants