search

mattes / migrate

Database migrations. CLI and Golang library.
Other
2.29k stars 326 forks source link

Advisory unlock error with Postgres #296

Open Teddy-Schmitz opened 7 years ago

Teddy-Schmitz commented 7 years ago

Hi,

First, thanks for this library its been very useful to us. I believe we have found a bug in how the postgres driver does the unlock. It doesn't actually check if the unlock was successful. Postgres doesn't return an error, just true or false and this is not checked in the code.

https://github.com/mattes/migrate/blob/5b98c13eff7657ab49a1a5f705b72f961d7fc558/database/postgres/postgres.go#L148

This should be scanned into a bool just as the lock code does and checked. Also since we don't have the same session it will never actually be successful since the unlock needs to be called within the same session. The only reason this isn't a bigger issue is because I imagine most people close the connection after migration which will automatically release the lock.

It looks like this can be fixed in go1.9 based on your comment in #274 .

I'm happy to make a PR to add the check for if the unlock was successful or not but I'm unsure if you would actually want to abort the migration at that point since it was already successful, and hopefully it will be fixed whenever #274 is fixed as well.

Thanks!

mattes commented 7 years ago

I as well think #274 is blocking this. Thanks for finding this.

Teddy-Schmitz commented 7 years ago

You're welcome, any thoughts on adding the Go 1.9 feature to migrate? Maybe behind a compilation flag to support older versions?