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

feature: Add datatype for uuid.UUID (Google UUID) #259

Merged
merged 3 commits into from
Jul 12, 2024

Conversation

omkar-foss
Copy link
Contributor

feature: Add datatype for uuid.UUID (Google UUID)

  • [*] Do only one thing
  • [*] Non breaking API changes
  • [*] Tested

What did this pull request do?

This PR adds datatype for uuid.UUID along with supporting tests.

User Case Description

The Google UUID package is a very popular package among Go developers. Since it's used so widely and so often, it'll be great to abstract it out as a separate gorm datatype. This came up as per the discussion on this Gorm PR with @a631807682 and @iTanken.

This PR adds datatype for uuid.UUID along with supporting tests.
@omkar-foss
Copy link
Contributor Author

Hey @jinzhu, hope you're doing great. It looks like the MariaDB (mariadb:latest, 1.19, ubuntu-latest) runner in GitHub Actions keeps failing with Failed to initialize container mariadb:latest error (example). This looks to be the case for several other PRs merged to master as well (example).

Any idea what's the issue there? Is it possible to disable the PR check for MariaDB runner until it's fixed?

@jinzhu jinzhu merged commit febbbcc into go-gorm:master Jul 12, 2024
8 of 9 checks passed
@omkar-foss omkar-foss deleted the add-uuid-datatype branch July 15, 2024 06:02
@pcfreak30
Copy link

@omkar-foss I just saw this and think its awesome as im currently using a custom UUID datatype.

One question I have is, is there a reason for storing this as a string and not the raw binary? I store as bytes for efficiency.

@omkar-foss
Copy link
Contributor Author

omkar-foss commented Aug 2, 2024

@omkar-foss I just saw this and think its awesome as im currently using a custom UUID datatype.

That's great to know @pcfreak30 😁

One question I have is, is there a reason for storing this as a string and not the raw binary? I store as bytes for efficiency.

Great question. The custom datatype UUID added in this PR is intended to be a wrapper around Google's uuid.UUID - which currently maps the UUID values to strings (code here). There's also a past discussion referring to the use of bytes for efficiency in Google's uuid.UUID, check it out here - google/uuid#20.

Having said that, I suppose it'll indeed be a good idea to have a separate custom datatype for UUID being stored as bytes (can call it BinUUID or ByteUUID or similar), so users can use either one as per preference. Thanks for bringing this up, I'll raise a PR for this soon.

Update: I've raised PR #264 to add a new datatype BinUUID which stores uuid as binary (byte) array in the database. Hope this will be useful.

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.

3 participants