Skip to content
New issue

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

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

Already on GitHub? # to your account

Generate UUID primary keys in the database #578

Conversation

matthewmcgarvey
Copy link
Member

Fixes #122

When a user creates a migration that uses a UUID primary key, the migration will automatically set the default value to be gen_random_uuid() which comes from the pgcrypto extension. If they do not have that extension enabled, the migration will fail saying that gen_random_uuid() is an unknown function. They will need to add enable_extension "pgcrypto" in their migration before creating the table to get it working.

This does not remove or change how Avram currently sets the id value itself before inserting the record into the database

{% if primary_key_type.id == UUID.id %}
before_save set_uuid
def set_uuid
{% primary_key_name = type.resolve.constant("PRIMARY_KEY_NAME") %}
{{ primary_key_name.id }}.value ||= UUID.random()
end
{% end %}

We will definitely need to consider how we want to make that change but it doesn't need to be considered right now since there's no harm in doing both.

Copy link
Member

@jwoertink jwoertink left a comment

Choose a reason for hiding this comment

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

Interesting. I've never used the pgcrypto before, only ever used the uuid-ossp one. Taking a look at the docs, it looks like this is supported back to PG 9.X at least, and the uuid-ossp docs actually say

Note: If you only need randomly-generated (version 4) UUIDs, consider using the gen_random_uuid() function from the pgcrypto module instead.

So I think this is great 👍

@jkthorne
Copy link
Contributor

I have to do this all the time in Rails and Lucky. Thanks for this!

@matthewmcgarvey matthewmcgarvey added the BREAKING CHANGE This will cause a breaking change label Dec 28, 2020
@matthewmcgarvey
Copy link
Member Author

@jwoertink I'm not sure if I made it clear but this would be a breaking change since migrations won't work that have UUID primary keys until they enable the extension. Are you ok with this going in?

@jwoertink
Copy link
Member

Oh, like existing migrations where people might rollback or whatever? If you happen to know a way we can catch that, or show a nice error like "You need to enable the pgcrypto extension to use UUID. Here's how: ....". If not, then I think it's fine to go in.

@matthewmcgarvey matthewmcgarvey force-pushed the uuid-primary-key-default-value branch from 571cda9 to 6d2129c Compare December 28, 2020 21:35
@matthewmcgarvey
Copy link
Member Author

After looking into it, there's no simple way to add a helpful error to tell users to enable the extensions. For right now, I'm fine with the way it works. If it becomes a problem we can add it. It would be a rescue somewhere that looks for a message like function gen_random_uuid() does not exist and then wrap it. It would have to be a general rescue

@matthewmcgarvey matthewmcgarvey merged commit d6e5845 into luckyframework:master Dec 28, 2020
@matthewmcgarvey matthewmcgarvey deleted the uuid-primary-key-default-value branch December 28, 2020 21:41
@matthewmcgarvey
Copy link
Member Author

Users with existing tables without the default will need to create a migration that does this for every table in the database that needs it

execute("ALTER TABLE users ALTER COLUMN id SET DEFAULT gen_random_uuid();")

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
BREAKING CHANGE This will cause a breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Generate UUID v4 in Postgres
3 participants