-
Notifications
You must be signed in to change notification settings - Fork 1.5k
generate uuid v7 #4681
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
generate uuid v7 #4681
Conversation
|
I wonder if we want to have a 2 arity function to let users control the sub millisecond monotonicity guarantee. For example from what I know Postgres and Maria DB use 12 extra timestamp bits to include the nano seconds. Or maybe we want to consider just using nanoseconds in the 1 arity function to be more inline with these databases |
lib/ecto/uuid.ex
Outdated
| """ | ||
| @spec generate() :: t | ||
| def generate(), do: encode(bingenerate()) | ||
| def generate(), do: encode(bingenerate(@default_version)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| def generate(), do: encode(bingenerate(@default_version)) | |
| def generate(version \\ @default_version), do: encode(bingenerate(version)) |
And similar below.
lib/ecto/schema.ex
Outdated
| section for more info). Primary keys are automatically set up for embedded | ||
| schemas as well, defaulting to `{:id, :binary_id, autogenerate: true}`. | ||
| This will generate the default UUID v4. You can use UUID v7 instead by setting | ||
| the primary key to `{:id, :binary_id, autogenerate: {Ecto.UUID, :generate, [:v7]}}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's also document this in Ecto.UUID's module doc.
|
@greg-rychlewski in such cases, we can provide a :v7_nano in the future? |
|
I know that @ryanwinchester implemented / improved uuid v7 for |
In
Yes. In postgres at least, it's monotonic and stepped by a certain minimum number of bits as per section 6.2, method 3. I created a separate UUIDv7 library that does this as well. I think if we're adding UUIDv7 to Ecto, it would be useful if we did this to match the way the most used database(s) do it. Or at least the option to. |
@ryanwinchester I agree. The question is if we do it by default or opt-in. Do you have any thoughts on the matter? In any case, I think we can merge this now and you could contribute your nano implementation next. Would that work for you? Or would prefer for it to be baked in right now? |
Personally, I would prefer it to be default as it is in Postgres, but I don't know what most people would prefer.
Maybe instead of i.e. we could have (there are 3 optional methods for increased precision + monotonicity in the RFC, and I can contribute method 3) |
Yes, regardless of what we do here, I think options would be better indeed, thank you. @dkuku, can you please adjust accordingly? Also, can you please go ahead and change Ecto.Schema to make it so that, if |
It started to work with |
lib/ecto/type.ex
Outdated
| to `field` with the `:autogenerate` flag. | ||
| """ | ||
| @callback autogenerate() :: term() | ||
| @callback autogenerate(term) :: term() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it could be a Keyword but I see the functions in this module mostly operate on terms so I keep it consistent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can skip this change I think... since theoretically with the MFA you can call it anything or even have higher arity?
lib/ecto/uuid.ex
Outdated
| @spec generate() :: t | ||
| def generate(), do: encode(bingenerate()) | ||
| @spec generate(options) :: t | ||
| def generate(opts \\ [@default_options]), do: encode(bingenerate(opts)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably don't need the @default_options and could just use an empty list since you supply a default version with Keyword.get/3
lib/ecto/uuid.ex
Outdated
| @spec bingenerate() :: raw | ||
| def bingenerate() do | ||
| def bingenerate(), do: bingenerate(@default_options) | ||
|
|
||
| @doc """ | ||
| Generates a uuid with the given options in binary format. | ||
| """ | ||
| @spec bingenerate(options) :: raw | ||
| def bingenerate(opts) do | ||
| case Keyword.get(opts, :version, @default_version) do | ||
| 4 -> bingenerate_v4() | ||
| 7 -> bingenerate_v7() | ||
| _ -> raise ArgumentError, "unknown UUID version: #{inspect(opts[:version])}" | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same idea as generate, we don't need the default options
| @spec bingenerate() :: raw | |
| def bingenerate() do | |
| def bingenerate(), do: bingenerate(@default_options) | |
| @doc """ | |
| Generates a uuid with the given options in binary format. | |
| """ | |
| @spec bingenerate(options) :: raw | |
| def bingenerate(opts) do | |
| case Keyword.get(opts, :version, @default_version) do | |
| 4 -> bingenerate_v4() | |
| 7 -> bingenerate_v7() | |
| _ -> raise ArgumentError, "unknown UUID version: #{inspect(opts[:version])}" | |
| end | |
| end | |
| @doc """ | |
| Generates a uuid with the given options in binary format. | |
| """ | |
| @spec bingenerate(options) :: raw | |
| def bingenerate(opts \\ []) do | |
| case Keyword.get(opts, :version, @default_version) do | |
| 4 -> bingenerate_v4() | |
| 7 -> bingenerate_v7() | |
| version -> raise ArgumentError, "unknown UUID version: #{inspect(version)}" | |
| end | |
| end |
josevalim
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please address @ryanwinchester's feedback and we can ship it!
Thank you all! ❤️
|
💚 💙 💜 💛 ❤️ |
|
Thank you @ryanwinchester and @dkuku, looking forward to moving to ecto for generating uuidv7 instead of relying on uniq |
Add uuid v7 generator.
Also the formatter updated the files - I think it's because it was not touched for long time.
example uuid generated by some random page from google:
019a9da1-53b4-7f77-9609-140ad8880bd2and one generated with ecto: