Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow 64bit migration versions #152

Closed
wants to merge 2 commits into from

Conversation

ankhers
Copy link

@ankhers ankhers commented Feb 4, 2021

This should allow the versions to be parsed and inserted into the database.

I still need to add a command to the CLI to generate a migration for each supported database.

fixes #83

@ankhers ankhers marked this pull request as draft February 4, 2021 02:50
@ankhers ankhers changed the title [WIP] Allow 64bit migration versions Allow 64bit migration versions Feb 4, 2021
@ankhers ankhers force-pushed the allow_64bit_migration_versions branch 2 times, most recently from 3da0b56 to 4ec140e Compare February 11, 2021 17:25
There is no FromSQL implementation for u64

Because we changed the version from i32 to i64, the checksums will change.
@ankhers ankhers force-pushed the allow_64bit_migration_versions branch from 4ec140e to 5554e40 Compare February 11, 2021 17:54
@ankhers
Copy link
Author

ankhers commented Feb 15, 2021

I'm not really sure what the best command name to generate the migration. I'm open to suggestions if anyone has any.

@jxs
Copy link
Member

jxs commented Feb 16, 2021

Hi, and thanks for starting this. I would suggest update-schema?

@ankhers
Copy link
Author

ankhers commented Feb 22, 2021

So, I have been thinking about the auto generated migration and I am wondering whether this is something we want to do. Some people will prefer to have their migrations written in rust, others will prefer sql. Even then, there is the difference between versioned and unversioned migrations. Should we just have the ability to generate all of these different types of migrations, or should there just be a note in the changelog and readme stating to migrate the table?

@ankhers
Copy link
Author

ankhers commented Feb 22, 2021

Or maybe instead of creating a migration, we could just have refinery take care of it? Similar to how refinery will automatically create the schema table for you.

@jxs
Copy link
Member

jxs commented Feb 28, 2021

Or maybe instead of creating a migration, we could just have refinery take care of it? Similar to how refinery will automatically create the schema table for you.

can that be done with sql standard? If so yeah definitely, thanks!

@ankhers
Copy link
Author

ankhers commented Mar 10, 2021

So this is the functionality that you would like the keep? How would I go about applying the "older" migrations?

@jxs
Copy link
Member

jxs commented Mar 17, 2021

sorry, did not understand the question. How does this affect older migrations?

@ankhers
Copy link
Author

ankhers commented Mar 17, 2021

Really sorry about this. I don't really understand how I managed it, but I meant to reply to the most recent comment on #83. Let me repost question there just so that everything is clear.

@jxs jxs force-pushed the main branch 13 times, most recently from ed1aa79 to c5e17c9 Compare July 10, 2021 23:33
@taqtiqa-mark
Copy link
Contributor

ON 32 bit systems this should fallback to characters, or should these longer 'version' number schemes just be character/String consistently.

I also wonder if we wouldn't have less headaches on smaller platforms if we just used usize and fall back to char/String when the versions number is 'too-big' for the system refinery is running on?

@jxs jxs force-pushed the main branch 6 times, most recently from 8e1f0ff to 3650a0d Compare August 16, 2022 19:07
@lf-
Copy link

lf- commented Oct 4, 2022

ON 32 bit systems this should fallback to characters, or should these longer 'version' number schemes just be character/String consistently.

Bitness should not matter as long as the database supports INT8, which seems like a reasonable baseline requirement to be usable as a database anyway.

I'm super excited about this change because I just wrote unfortunate code that puts a Unix epoch time into the version number.

@lf-
Copy link

lf- commented Oct 4, 2022

Actually, since Refinery checksums are 64 bits, it already requires that the DB can store a 64 bit integer.

@jxs
Copy link
Member

jxs commented Sep 13, 2023

going to close this out of lack of activity, feel free to re-open if you or anyone wants to work on this again.
Thanks!

@jxs jxs closed this Sep 13, 2023
@mbrgm
Copy link

mbrgm commented Jan 18, 2024

@jxs What's missing for this to be merged?

Would a feature flag be an acceptable solution to not risk breaking any existing dependent code out there?

@jxs
Copy link
Member

jxs commented Jan 21, 2024

Hi @mbrgm this PR was stale for more than a year before being closed, so I don't know what was missing and how would this PR be compatible with the current codebase but feel free to fork the branch and work on the feature, if you want any guidance ping me.
Cheers :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The refinery_schema_history version colum is too small for time based versions.
5 participants