make migration id bigserial

[jkthorne]

Cockrochdb uses a bigserial and this breaks with using the migrator. This is compatible with both postgresql and cockroachdb.

[jwoertink]

I'm cool with this change, but one thing I'm wondering is what happens for existing apps that try to run a migration? Crystal-db will try to pull out the serial field, and cast to Int64, right? Will the upcast still work, or will that tank? 🤔

Or we say this is a breaking change, and then write up a little migration that updates the existing column....

[jwoertink]

side note, ignore the ameba failures. Everything has been updating to 0.14 which seems to have some issues.

[robacarp]

It's been a few compiler versions, but I don't think upcasting works. If it expects an Int64 and gets an Int32, it doesn't work. That makes this a breaking change, but I think it's still 🆗.

[matthewmcgarvey]

I'd rather not have a breaking change here. We can create an alias alias MigrationId = Int32 | Int64 and that should work without breaking existing apps

[jwoertink]

I've never used Cockroachdb, but I'm under the impression the error is more on the db side where it's defined as serial vs bigserial. If that's the case, would adding that alias make a difference? @wontruefree can you confirm which side is having the issue on this? Like, is cockroach failing because the table defines serial, and it doesn't know how to handle that, or is it that it returns In32 on the crystal side?

[jkthorne]

This is a cockroachdb issue and one that I have brought up with there team. Here is an old issue linked here.
cockroachdb/cockroach#55864

Migrating existing application is a concern for sure. I think the idea of an alias can solve the problem of older applications.

I think a concern like this eventually migrate to avram so different database adapters could handle this.

[jwoertink]

Ah, so if we had the alias, and old apps used serial, and new apps used bigserial, then it would work for both? If that's the case, then let's add in that alias, and then update the return type to point to that alias. How does that sound @matthewmcgarvey ?

[stephendolan]

Just tested this by:

• Creating a new default 0.26 Lucky app via lucky init.custom my_app
• Running ./script/setup and verifying that the migrations table id was of type integer
• Adding a shard.override.yml with @wontruefree's fork of avram, on the master branch
• Verifying that the content from this PR was indeed in lib/avram
• Running lucky gen.model Post body:String and then lucky db.migrate
• Migration ran successfully, posts table existed in the database

[matthewmcgarvey]

Thanks for this @wontruefree 🎉