| From: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
|---|---|
| To: | Tristan Partin <tristan(at)partin(dot)io> |
| Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Support UUIDv6 in uuid_extract_timestamp() |
| Date: | 2026-08-04 20:42:29 |
| Message-ID: | CAD21AoDu46SipzFBZHxpTC+83W=Z=rqcYAJ1SMiHVNJazeZF5g@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Mon, Aug 3, 2026 at 3:53 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> On Mon, Aug 3, 2026 at 12:55 PM Tristan Partin <tristan(at)partin(dot)io> wrote:
> >
> > 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.
> >
>
> Thank you for updating the patches.
>
> Yeah, the current explanation seems better as the terms like
> "time_high" matches ones the RFC uses. I'll push these patches barring
> any objections.
Pushed.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Bharath Rupireddy | 2026-08-04 20:55:00 | Re: explain plans for foreign servers |
| Previous Message | Masahiko Sawada | 2026-08-04 20:25:52 | Re: Add a hook for handling logical decoding messages on subscribers. |