Re: UUID v7 - Mailing list pgsql-hackers
From | Peter Eisentraut |
---|---|
Subject | Re: UUID v7 |
Date | |
Msg-id | 2cf9eaf5-057f-4dfa-ae4d-9b23c5339abe@eisentraut.org Whole thread Raw |
In response to | Re: UUID v7 ("Andrey M. Borodin" <x4mmm@yandex-team.ru>) |
Responses |
Re: UUID v7
|
List | pgsql-hackers |
On 20.03.24 19:08, Andrey M. Borodin wrote: >> On 19 Mar 2024, at 13:55, Peter Eisentraut <peter@eisentraut.org> wrote: >> >> On 16.03.24 18:43, Andrey M. Borodin wrote: >>>> On 15 Mar 2024, at 14:47, Aleksander Alekseev <aleksander@timescale.com> wrote: >>>> >>>> +1 to the idea. I doubt that anyone will miss it. >>> PFA v22. >>> Changes: >>> 1. Squashed all editorialisation by Jelte >>> 2. Fixed my erroneous comments on using Method 2 (we are using method 1 instead) >>> 3. Remove all traces of uuid_extract_variant() >> >> I have committed a subset of this for now, namely the additions of uuid_extract_timestamp() and uuid_extract_version(). These seemed mature and agreed upon. You can rebase the rest of your patch on top of that. > > Great! Thank you! PFA v23 with rebase on HEAD. I have been studying the uuidv() function. I find this code extremely hard to follow. We don't need to copy all that documentation from the RFC 4122bis document. People can read that themselves. What I would like to see is easy to find information what from there we are implementing. Like, - UUID version 7 - fixed-length dedicated counter - counter is 18 bits - 4 bits are initialized as zero That's more or less all I would need to know what is going on. That said, I don't understand why you say it's an 18 bit counter, when you overwrite 6 bits with variant and version. Then it's just a 12 bit counter? Which is the size of the rand_a field, so that kind of makes sense. But 12 bits is the recommended minimum, and (in this patch) we don't use sub-millisecond timestamp precision, so we should probably use more than the minimum? Also, you are initializing 4 bits (I think?) to zero to guard against counter rollovers (so it's really just an 8 bit counter?). But nothing checks against such rollovers, so I don't understand the use of that. The code code be organized better. In the not-increment_counter case, you could use two separate pg_strong_random calls: One to initialize rand_b, starting at &uuid->data[8], and one to initialize the counter. Then the former could be shared between the two branches, and the code to assign the sequence_counter to the uuid fields could also be shared. I would also prefer if the normal case (not-increment_counter) were the first if branch. Some other notes on your patch: - Your rebase duplicated the documentation of uuid_extract_timestamp and uuid_extract_version. - PostgreSQL code uses uint64 etc. instead of uint64_t etc. - It seems the added includes #include "access/xlog.h" #include "utils/builtins.h" #include "utils/datetime.h" are not needed. - The static variables sequence_counter and previous_timestamp could be kept inside the uuidv7() function.
pgsql-hackers by date: