yogthos / migratus

MIGRATE ALL THE THINGS!
651 stars 95 forks source link

Fix error if any migration lacked an applied date #237

Closed svdm closed 1 year ago

svdm commented 1 year ago

The sort on :applied in migratus.database/completed-ids* did not handle nil, meaning the presence of one or more migrations lacking an applied timestamp would cause it to break. Any sql migrations pre-0.9.1 will not have an applied value unless users have retroactively set it, and the schema allows NULLs, so migratus cannot assume applied is not nil.

The fix is simple because we have compare, which is essentially .compareTo-but-handles-nil. Any nil values will be sorted to the end, which is correct for the intended purpose here.

Current tests wouldn't catch this because they migrate up a database from scratch without legacy records. I have looked for a relevant test to update/add, but there's no integration test with an existing migrations table and it seems like a bunch of work to add one.

yogthos commented 1 year ago

Thanks, just pushed out 1.4.9 with the fix.