Migrations don't work past the number 9 when using an incremental integer

[darrencauthon]

I've been using indexing my migration scripts with an incremental integer. After migrating past my tenth script, the system stopped working because dotnetmigrations thought I was only on version 9.

The problem is the select statement here:

const string command = "SELECT MAX([version]) FROM [schema_migrations]"; https://github.com/jpoehls/dotnetmigrations/blob/master/src/DotNetMigrations.Core/BaseClasses/DatabaseCommandBase.cs#L73

Since the version field is nvarchar(14), "9" is considered greater than 10, 11, etc.

The fast fix is to manually change the property in the database to integer, but that doesn't help people who use this library. There are a number of ways to solve this issue, but since this logic is rooted deep in a base class I'm not sure which method should be used.

[JamesEggers1]

Thanks Darren. This is an issue we're looking into and should be part of a decent sized update in the near future. Two largest challenges with this deals with people who may want to change their versioning strategy for some reason as well as implementing a solution that works on all major database platforms. We'll get this fixed as soon as we are able to.

[darrencauthon]

Thanks James.

Maybe a faster fix for this bug would be to change the "SELECT Max..." call to a SQL statement that will sort the numbers properly, even though they are saved as nvarchar(14). Then you can have it both ways, since nothing will change in the database and only those who have made the switch to ints will get the new logic. It would be a good refactor, too, so the SQL statement can be in one place instead of many.

[jpoehls]

This has been fixed in 6d4cdd9 by changing the [schema_migration].[version] column's data type from [varchar] to [int]. Note that you will have to manually change this column's data type in your existing databases by running the following SQL to alter the column. The fix in DNM will only affect new databases being migrated.

Here is the SQL you need to manually run: ALTER TABLE [schema_migrations] ALTER COLUMN [version] [int] NOT NULL

Oh yeah, this is in master now and will be included in the v0.83 release when that goes out.

[darrencauthon]

Thanks Josh, I'm glad this ended up to be the solution. I wasn't sure if the nvarchar(14) was for some reason I wasn't seeing, like a naming scheme where the value wasn't numeric.

[jpoehls]

Ok. So after reading your reply I remembered why it was nvarchar(14). The UTC timestamp version numbers are too big to fit into an int column! So yeah, v0.83 and 84 are completely broke for the utc_time and local_time versioning strategies.

I just push v0.85 with a fix for this although it does require some manual modifications to the [schema_migrations] table as documented in the changelog. Basically I added an [id] [int] identity(1,1) primary key column to use to find the max version number. This works since migrations are always executed in sequential order so the order of the IDs will match the order of the actual version numbers.

Please let me know if you find any issues. I'm hoping this release is fairly stable now.

[darrencauthon]

Hmm... yeah, I think that will work. My only concern with the table update is that it makes it a little fragile. If, for example, a record was added manually to the table and then deleted, the ids will be thrown off permanently.

Perhaps one alternative is to make the [id] just a keyed int where the next value is pulled from the migration scripts instead of the identity(1,1). Another alternative is to keep it as nvarchar(14), but change the Select Max([id]) call to something like "Select Max(Convert(long, [id]))" or something equivalent.

[jpoehls]

If a record was added manually and then deleted the IDs then there would be a gap in the IDs, but they would still be in the same sequential order that the migration scripts were executed in. The gaps wouldn't matter.

If somehow this becomes a real issue down the line we can figure something out then. Supporting manual modifications of the underlying tracking tables is not something I'm going to pursue without some very good cause.