-
Notifications
You must be signed in to change notification settings - Fork 257
Add UUID version 7 as the default guid generator #3249
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
Changes from 1 commit
c4139f7
121dcc4
1ddc4ca
a667a91
72810e3
85c1b84
01e6dee
6393e39
2ccd6a4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| using System.Runtime.InteropServices; | ||
|
|
||
| namespace Npgsql.EntityFrameworkCore.PostgreSQL.ValueGeneration; | ||
|
|
||
| /// <summary> | ||
| /// Generates sequential <see cref="Guid" /> values according to the UUID version 7 specification. | ||
| /// Will be updated to use Guid.CreateVersion7 when available. | ||
| /// </summary> | ||
| public class UUid7ValueGenerator : ValueGenerator<Guid> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: The .NET team has chosen to stick to avoid using multiple synonyms, hence There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question: All generators in the current package are prefixed with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moreover, they are marked as internal APIs. Should we rather be consistent with them or with EF Core's own
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. EF's GuidValueGenerator is fully public since it's meant to be referenceable by external providers, i.e. it's infrastructure; whereas types inside EFCore.PG aren't... So I think it's indeed better to make the generator pubternal (by moving it into the Internal namespace) - we don't want people to start using it for whatever purpose, it's just an internal implementation detail of the provider. Given that it would be pubternal, the naming doesn't matter too much. I have a slight preference for UUID7, since that's the definition of that particular format; the .NET type which holds UUIDs (of any sort) happens to be Guid, but the actual format is UUID7 I think. But no strong feelings. Re the Npgsql prefix, no strong feelings there either (again, it would be a pubternal type anyway). Since there's nothing particular Npgsql-specific about it - it just generates UUID7s, I'd personally leave it out. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's have the prefix, for consistency with similar existing types in the package, and to reinforce the pubternal-ness some more. |
||
| { | ||
| private const byte Variant10xxValue = 0x80; | ||
| private const ushort Version7Value = 0x7000; | ||
| private const ushort VersionMask = 0xF000; | ||
| private const byte Variant10xxMask = 0xC0; | ||
|
|
||
| /// <summary> | ||
| /// Gets a value to be assigned to a property. | ||
| /// </summary> | ||
| /// <param name="entry">The change tracking entry of the entity for which the value is being generated.</param> | ||
| /// <returns>The value to be assigned to a property.</returns> | ||
| public override Guid Next(EntityEntry entry) | ||
| { | ||
| Span<byte> guidBytes = stackalloc byte[16]; | ||
| var succeeded = Guid.NewGuid().TryWriteBytes(guidBytes); | ||
| var unixms = DateTimeOffset.UtcNow.ToUnixTimeMilliseconds(); | ||
| Span<byte> counterBytes = stackalloc byte[sizeof(long)]; | ||
|
ChrisJollyAU marked this conversation as resolved.
Outdated
|
||
| MemoryMarshal.Write(counterBytes, in unixms); | ||
|
|
||
| if (!BitConverter.IsLittleEndian) | ||
| { | ||
| counterBytes.Reverse(); | ||
| } | ||
|
|
||
| //unix ts ms - 48 bits (6 bytes) | ||
| guidBytes[00] = counterBytes[2]; | ||
| guidBytes[01] = counterBytes[3]; | ||
| guidBytes[02] = counterBytes[4]; | ||
| guidBytes[03] = counterBytes[5]; | ||
| guidBytes[04] = counterBytes[0]; | ||
| guidBytes[05] = counterBytes[1]; | ||
|
|
||
| //UIDv7 version - first 4 bits (1/2 byte) of the next 16 bits (2 bytes) | ||
| var _c = BitConverter.ToInt16(guidBytes.Slice(6, 2)); | ||
| _c = (short)((_c & ~VersionMask) | Version7Value); | ||
| BitConverter.TryWriteBytes(guidBytes.Slice(6, 2), _c); | ||
|
|
||
| //2 bit variant | ||
| //first 2 bits of the next 64 bits (8 bytes) | ||
| guidBytes[8] = (byte)((guidBytes[8] & ~Variant10xxMask) | Variant10xxValue); | ||
| return new Guid(guidBytes); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Gets a value indicating whether the values generated are temporary or permanent. This implementation | ||
| /// always returns false, meaning the generated values will be saved to the database. | ||
| /// </summary> | ||
| public override bool GeneratesTemporaryValues | ||
| => false; | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.