On Mon, Aug 3, 2026 at 3:53 PM Masahiko Sawada <[email protected]> wrote:
>
> On Mon, Aug 3, 2026 at 12:55 PM Tristan Partin <[email protected]> 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 <[email protected]> 
> > > wrote:
> > >>
> > >> On Wed, Jun 24, 2026 at 1:10 PM Tristan Partin <[email protected]> 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


Reply via email to