initial implementation of UUIDv6 and UUIDv7 based on RFC Draft Rev 2 #93
Conversation
theckman
force-pushed
the
draft-peabody-dispatch-new-uuid-format-01
branch
2 times, most recently
from
b2f60a0
to
2b49ae5
Compare
Codecov Report
@@ Coverage Diff @@
## master #93 +/- ##
==========================================
+ Coverage 98.87% 99.27% +0.39%
==========================================
Files 4 4
Lines 356 549 +193
==========================================
+ Hits 352 545 +193
Misses 3 3
Partials 1 1
Continue to review full report at Codecov.
|
theckman
force-pushed
the
draft-peabody-dispatch-new-uuid-format-01
branch
from
2b49ae5
to
72e1726
Compare
|
So I updated my commit but codecov did not update the comment to indicate I'd fixed the coverage decrease... |
|
There we go... |
| binary.BigEndian.PutUint16(u[4:], uint16(timeNow>>12)) // set time_mid | ||
| binary.BigEndian.PutUint16(u[6:], uint16(timeNow&0xfff)) // set time_low (minus four version bits) | ||
| binary.BigEndian.PutUint16(u[8:], clockSeq&0x3fff) // set clk_seq_hi_res (minus two variant bits) | ||
|
|
There was a problem hiding this comment.
Do we need to set u[9:] to clk_sec_low_res
There was a problem hiding this comment.
This does that implicitly. uint16 is represented as two bytes, so we use the BigEndian binary encoder to put the uint16 starting at u[8]. So it writes both bytes out, covering the hi and lo bits.
There was a problem hiding this comment.
I've approved the PR, but maybe a comment to that effect would be nice!
| binary.BigEndian.PutUint16(u[8:], clockSeq&0x3fff) // set clk_seq_hi_res (minus two variant bits) | ||
|
|
||
| u.SetVersion(V6) | ||
| u.SetVariant(VariantRFC4122) |
There was a problem hiding this comment.
is this variant correct? It's looking for the value, but can't remember of go has iotas 0 indexed (which would make this 1) or if this is indeed the right value.
There was a problem hiding this comment.
@cameracker SetVariant does the translation.
| d |= (msec << 16) // set msec field | ||
| d |= (uint64(seq) & 0xfff) // set seq field | ||
|
|
||
| binary.BigEndian.PutUint64(u[:], d) |
There was a problem hiding this comment.
is grouping it this way going to result in the msec and seq bits being stored BigEndian? That may be a deviation from the spec, because only the nano bits are stored as big endian (at least according to my reading).
Note, this concern applies to all the v7 generators.
There was a problem hiding this comment.
The original spec (RFC 4122) specifies that values should be in network byte order (Big Endian):
The fields are encoded as 16 octets, with the sizes and order of the fields defined above, and with each field encoded with the Most Significant Byte first (known as network byte order). Note that the field names, particularly for multiplexed fields, follow historical practice.
This draft doesn't make any changes to that.
|
I guess one more question, do you think we'd benefit from a test that verifies these are sortable as we expect? |
|
@cameracker I could probably do that in one of the tests I currently have where I generate a bunch of UUIDs to actually exercise the clock sequence generator. I could fail the test if the previous UUID isn't I'll add that in tonight. |
theckman
force-pushed
the
draft-peabody-dispatch-new-uuid-format-01
branch
2 times, most recently
from
10d97a6
to
9f8ac2f
Compare
theckman
changed the title
There is currently an RFC draft in progress[1] to add three new UUID formats, versions 6, 7, and 8, that change the internal data format so that the generated UUIDs are k-sortable. Version 8 is a special UUID format designed for future implementations or extensions, and as a result isn't implemented here. Because this implementation is based on a draft RFC, they may be subject to change in later revisions. As such, these new functions/methods include a comment indicating they do not yet fall under the API compatibility guarantee of the project, and changes to meet new revisions to the spec will be done in minor releases. [1] https://datatracker.ietf.org/doc/html/draft-peabody-dispatch-new-uuid-format-02
theckman
force-pushed
the
draft-peabody-dispatch-new-uuid-format-01
branch
from
9f8ac2f
to
03e219b
Compare
|
Confirmed we are compatible with Revision 2 of the RFC Draft. Updated the PR to reflect that, and added the tests. Ready for review+merge. |
|
Do we need to update the README for this or are we waiting for stabilization? |
|
Thanks @johanbrandhorst , I took a swing at that here: #94 |
There is currently an RFC draft in progress[1] to add three new UUID formats,
versions 6, 7, and 8, that change the internal data format so that the generated
UUIDs are k-sortable. Version 8 is a special UUID format designed for future
implementations or extensions, and as a result isn't implemented here.
Because this impelmentation is based on a draft RFC, they may be subject to
change in later revisions. As such, these new functions/methods include a
comment indicating they do not yet fall under the API compatibility guarantee of
the project, and changes to meet new revisions to the spec will be done in minor
releases.
[1] https://datatracker.ietf.org/doc/html/draft-peabody-dispatch-new-uuid-format-02