Down migration on MariaDB fails if down file is empty

[vendion]

According to the documentation found here it states that having empty migrations is permitted by migrate:

The migration files are permitted to be empty, so in the event that a migration is a no-op or is irreversible, it is recommended to still include both migration files, and either leaving them empty or adding a comment as appropriate.

But in my experience this does not seem to be the case as trying to rollback a migration where the down file is empty results in the following error:

error: migration failed in line 0:  (details: Error 1065: Query was empty)

Steps to Reproduce
Steps to reproduce the behavior:

1. Create a new DB migration, leaving the down migration file empty.

$ migrate -ext migrations -dir migrations empty_down_test
$ echo "UPDATE foo SET bar='baz' WHERE bar='buzz';" > migrations/20190701103700_empty_down_test.migration

2. Preform the up migration then try to rollback:

$ migrate -path migrations -source "${MIGRATE_DB}" up
$ migrate -path migrations -source "${MIGRATE_DB}" down 1

3. See error

Expected Behavior
Since the down migration file was empty there should be no action taken other than updating the schema_migrations table to the new version and mark it as being in a clean state (as it already was before trying to rollback the migration).

Migrate Version
v4.4.0

Loaded Source Drivers
godoc-vfs, gcs, file, s3, github, gitlab, go-bindata

Loaded Database Drivers
stub, cockroach, cockroachdb, mongodb, postgresql, redshift, spanner, cassandra, clickhouse, crdb-postgres, mysql, postgres, sqlserver

Go Version
go version go1.11.5 linux/amd64

Additional context
This was tested using the prebuilt v4.4.0 binaries available on this project's releases page, and using MariaDB .5.5.60. A usable but annoying work around is to use the force command to skip the down migrations that are a noop like this case.

Has migrate's stance on empty migrations file changed, thus the docs are out dated or is this something else?

[dhui]

This is a bug and I've been able to reproduce it.

I'm exploring potential fixes for this issue. However, I'm pretty busy right now (that's why my response was delayed), so if you know where to make the fix in migrate/migrate.go, migrate/migration.go, and/or source/migration.go, please open a PR.
The fix should fix the issue for all source drivers without impacting performance. e.g. making unnecessary reads.

I believe that migrate currently handles missing migration files but doesn't handle empty ones.
So in the meanwhile, you can delete the down migrations if they're empty if you're blocked.

[dhui]

I have a fix ready and am working on tests. Don't have a ETA on when I'll finish the tests and ship the fix.

Also, I'm also not sure if this issue should be fixed. e.g. if users create a migration (empty by default) this fix will allow them to apply it. This is probably more of an issue for applying empty up migrations than empty down migrations.

[vendion]

Also, I'm also not sure if this issue should be fixed. e.g. if users create a migration (empty by default) this fix will allow them to apply it. This is probably more of an issue for applying empty up migrations than empty down migrations.

That's a good point without possibly separating the processing of up and down migrations that might be difficult and could cause other issues (have not thoroughly read the code but I do see your point).

I believe that migrate currently handles missing migration files but doesn't handle empty ones.
So in the meanwhile, you can delete the down migrations if they're empty if you're blocked.

I did test what happens when the migrate down command is ran with a missing down file and it did work.

So maybe this could be approached as less of a code issue and more of a documentation issue by updating this file https://github.com/golang-migrate/migrate/blob/master/MIGRATIONS.md to define an "empty migration" as being a missing file.

[dhui]

Since migrate should be as safe as possible by default, let's leave the current behavior. e.g. migrate will continue to run empty migration files and if the database doesn't like empty queries, it'll error out appropriately.
That way, users will know that they accidentally forgot to write a migration.

I've updated the MIGRATIONS.md file accordingly.

[vendion]

@dhui That is acceptable and the changes to the MIGRATIONS.md file makes it clear as far as the workaround any issues with an empty (zero byte file as you cleanly put it).

Thank you for taking a look into this.