Fixed issue with migrations

[bsidoruk]

Migrations were not running when more than one migration was present.

Description

Because version is stored as a bigint, there was an error when ecto_sql tried to fetch existing versions from the schema_migrations table. This PR updates the casting of columns so that all integers are cast as bigint rather than int. In addition, the column that is selected is re-aliased so that the Erlang ODBC library returns the columns returned. This allows the Mssqlex library to correctly determine the data type when transforming the returned values.

Motivation and Context

Needed to Elixir project to a MSSSQL db and ran into issues with migrations

How Has This Been Tested?

Set up migrations using phx.gen.schema. Multiple migrations can now be run, and data can be inserted an queried from the db.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

[whossname]

I'll have a play around with this at some stage in the next few days. How does it behave for users that have an existing database they are trying to interface with? Did you run the tests?

[bsidoruk]

Because I’m just changing the Cast to bigint, any values (in existing dbs) that would have been valid before will still be valid now. I didn’t manage to get the tests going even before I implemented my changes. I can take another run at it on Monday though.

[whossname]

Look at this bash script https://github.com/whossname/mssql_ecto/blob/ecto_3.0/bash_scripts/setup_test_db.sh

[whossname]

@bsidoruk I'm a bit concerned that this method could cause performance issues. A bigint uses twice as much memory compared to an int, although I don't know the low level details of how MSSQL server handles this.

Would you mind trying out the TDS library? From a technical stand point what they are doing makes more sense but I'm not sure how much progress they have made towards Ecto 3. This library relies on the Erlang odbc library, which isn't actively maintained and causes a bunch of the type errors. If that library is further along than this library it might make sense to deprecate this in favour of the TDS implementation.

These are my forks of that library, I'm playing around with the integration tests to see how it is going:

https://github.com/whossname/ecto_sql
https://github.com/whossname/tds

[bsidoruk]

I understand that but I actually just updated it to do what the Postgres adapter is doing in the ecto_sql library: https://github.com/elixir-ecto/ecto_sql/blob/17468a0f5b846f7d61de5a3ba255c14e150e3540/lib/ecto/adapters/postgres/connection.ex#L645. I looked into the TDS library, but there a few bugs with the adapter and it required using a fork of ecto_sql which I wasn't really a fan of. If it eventually got pulled into the main library I might look at it again. For now this is looking like the best solution. I'm currently pointing to this branch for my current project, and haven't ran into any other issues so far

[whossname]

This seems to break fragmented schemaless types. This works:

TestRepo.insert!(%Post{visits: 123})
assert [123] = TestRepo.all(from p in "posts", select: type(p.visits, :integer))

This doesn't:

TestRepo.insert!(%Post{visits: 123})
assert [123] = TestRepo.all(from p in "posts", select: type(fragment("visits"), :integer))

Otherwise it looks pretty good so far. I might open an issue in ecto_sql asking about the Ecto.Migration.SchemaMigration and why it casts to an integer instead of keeping the bigint. Need to do some more research first.

[whossname]

Ok research complete. I didn't figure out exactly what is going on with the type(field, type) syntax and tagged values, but I know enough to know that it is a design decision, not a bug. As far as I can tell type(field, type) tells ecto to parse the data using the Ecto.Query.Tagged struct. type is the ecto type, not database type. There is not ecto bigint type, :integer handles all of the integer sizes.

[bsidoruk]

I agree. It seems to defer to the adapter to decide how to treat integers. That is what this PR accomplishes. It will now treat :integer as bigint rather than int. It is now consistent with how the Postgres adapter was handling it. Additionally, it re-aliases the name of the column, since that is lost in the cast. This was causing an error with the Erlang ODBC library as it was no longer returning the column name when it was queried

[whossname]

I'm happy with this, but we need to at least try to fix that fragmented schemaless types issue. I'm working on a few other projects so I won't get to it for a few weeks. Are you planning on trying you hand at it?