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

dialect/sql/schema: file based type store #2644

Merged
merged 6 commits into from Jun 15, 2022
Merged

Conversation

masseelch
Copy link
Collaborator

@masseelch masseelch commented Jun 13, 2022

This PR adds support for a file based type storage when using versioned migrations. The file called .ent_types is written to the migration directory alongside the migration files and will be kept in sync for every migration file generation run.

In order to not break existing code, where the type storage might differ for different deployment, global unique ID mut be enabled by using a new option. This will also be raised as an error to the user when attempting to use versioned migrations and global unique ID.

Documentation will be added to this PR once feedback on the code is gathered.

Fixes #2587


I have discussed with Ariel already, that we will move the configuration for allocated pk ranges to the schema level. We haven't yet figured out, how to do this best, but it will come.

@masseelch masseelch requested review from a8m and rotemtam June 13, 2022 10:44
Copy link
Member

@a8m a8m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fantastic 💯

dialect/sql/schema/atlas.go Outdated Show resolved Hide resolved
dialect/sql/schema/atlas.go Show resolved Hide resolved
dialect/sql/schema/migrate.go Outdated Show resolved Hide resolved
@lagzi
Copy link

lagzi commented Jun 14, 2022

Hey, I'm getting validating migration directory: checksum mismatch when trying to create a new migration (.Diff(..)) because it seems that the .ent_types file changed but the checksum isn't updated.

@masseelch
Copy link
Collaborator Author

Hey, I'm getting validating migration directory: checksum mismatch when trying to create a new migration (.Diff(..)) because it seems that the .ent_types file changed but the checksum isn't updated.

Good catch. I will both add a test and a fix for that :-)

@masseelch
Copy link
Collaborator Author

masseelch commented Jun 14, 2022

@lagzi I changed added a commit, that should fix your problem. If it doesn't please ping me on Discord so I can help you resolve it 😊

This PR adds support for a file based type storage when using versioned migrations. The file called `.ent_types` is written to the migration directory alongside the migration files and will be kept in sync for every migration file generation run.

In order to not break existing code, where the type storage might differ for different deployment, global unique ID mut be enabled by using a new option. This will also be raised as an error to the user when attempting to use versioned migrations and global unique ID.

Documentation will be added to this PR once feedback on the code is gathered.
dialect/sql/schema/atlas.go Outdated Show resolved Hide resolved
dialect/sql/schema/migrate.go Outdated Show resolved Hide resolved
dialect/sql/schema/migrate.go Outdated Show resolved Hide resolved
@a8m
Copy link
Member

a8m commented Jun 14, 2022

Looks good 🚀

Please, fix the broken tests and I'll approve.

@masseelch
Copy link
Collaborator Author

masseelch commented Jun 15, 2022

Currently this PR is blocked on a checksum problem: .ent_types is added to the checksum as well and this has to be resolved on Atlas.

cc @a8m

@masseelch masseelch force-pushed the versioned/global-unique-id branch 2 times, most recently from 1e93cfe to 61b601f Compare June 15, 2022 09:18
@masseelch masseelch force-pushed the versioned/global-unique-id branch 2 times, most recently from 7142c03 to bef944e Compare June 15, 2022 12:27
Copy link
Member

@a8m a8m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the code but can't see WithDeterministicGlobalUniqueID. It's too long for me 🙈

@masseelch masseelch merged commit 7017cbc into master Jun 15, 2022
@masseelch masseelch deleted the versioned/global-unique-id branch June 15, 2022 14:10
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.

Versioned migrations do not work with GlobalUniqueID
3 participants