migrate icon indicating copy to clipboard operation
migrate copied to clipboard

Advisory unlock error with Postgres

Open Teddy-Schmitz opened this issue 7 years ago • 2 comments

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!

Teddy-Schmitz avatar Oct 19 '17 08:10 Teddy-Schmitz

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

mattes avatar Oct 20 '17 00:10 mattes

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

Teddy-Schmitz avatar Oct 20 '17 02:10 Teddy-Schmitz