| From: | "Tristan Partin" <tristan(at)partin(dot)io> |
|---|---|
| To: | "Masahiko Sawada" <sawada(dot)mshk(at)gmail(dot)com> |
| Cc: | "pgsql-hackers" <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Support UUIDv6 in uuid_extract_timestamp() |
| Date: | 2026-08-03 19:54:58 |
| Message-ID: | DKFKMFIHDE2J.1VKHJWW6HGXDP@partin.io |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Mon Aug 3, 2026 at 7:21 PM UTC, Masahiko Sawada wrote:
> On Wed, Jun 24, 2026 at 4:29 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>>
>> On Wed, Jun 24, 2026 at 1:10 PM Tristan Partin <tristan(at)partin(dot)io> wrote:
>> >
>> > UUIDv6 is the same as UUIDv1 with some fields reordered. We already
>> > supported UUIDv1, so let's add support for UUIDv6. Previously, calls to
>> > uuid_extract_timestamp() would return NULL for UUIDv6 values.
>> >
>> > Patch 2 only adds comments. I think the bit manipulation is worthy of
>> > comments, but others may disagree.
>>
>> Thank you for the patch!
>>
>> The patch seems reasonable to me. I'll review the patch in depth.
>>
>
> I've reviewed the v1 patches and the 0001 patch looks good to me. I
> have one comment for the 0002 patch:
>
> + /*
> + * UUIDv1 splits the 60-bit Gregorian timestamp into three fields that
> + * are *not* stored most-significant-first (see RFC 9562 sec. 5.1):
> + *
> + * time_low (bits 0-31) octets 0-3, the least significant 32 bits
> + * time_mid (bits 32-47) octets 4-5, the middle 16 bits
> + * time_high (bits 48-59) octet 6 low nibble + octet 7, the most
> + * significant 12 bits (octet 6 high nibble
> + * holds the version and is masked off)
> + *
> + * Reassemble the timestamp by shifting each field back to its place.
> + */
>
> pgindent destroys these field tables. We need to tell pgindent to
> leave the block alone by adding the dashed comment form:
>
> /*----------
> * UUIDv6 is a field-compatible reordering of UUIDv1 that stores the
> ...
> *----------
> */
>
> Or I guess that this part doesn't necessarily need to be such a
> format. Simply writing it like follow would work too:
>
> /*
> * UUIDv1 stores the 60-bit Gregorian timestamp in three fields that
> * are *not* in most-significant-first order (RFC 9562 sec. 5.1):
> * octets 0-3 hold the least significant 32 bits, octets 4-5 the
> * middle 16, and octet 6's low nibble plus octet 7 the most
> * significant 12. Octet 6's high nibble is the version, masked off
> * below. The shifts put each field back in its place.
> */
Thanks for the review. I went with option 1 to fix the comments in the
patch. I think it is a bit more readable than option 2. Included is
a re-spin of the first patch, but it is equivalent to v1. I tested
pgindent myself on v2, and it left the comment block alone.
--
Tristan Partin
PostgreSQL Contributors Team
AWS (https://aws.amazon.com)
| Attachment | Content-Type | Size |
|---|---|---|
| v2-0001-Support-UUIDv6-in-uuid_extract_timestamp.patch | text/x-patch | 3.8 KB |
| v2-0002-Add-comments-to-uuid_extract_timestamp.patch | text/x-patch | 3.0 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Matthias van de Meent | 2026-08-03 20:07:06 | Re: Resetting snapshots during the first phase of [CREATE |RE]INDEX CONCURRENTLY |
| Previous Message | Masahiko Sawada | 2026-08-03 19:20:29 | Re: Support UUIDv6 in uuid_extract_timestamp() |