| 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-03 22:53:06 |
| Message-ID: | CAD21AoDd4g1E1jd5e2rMC1L-hLr4PV2cC8J--FefD-YjRqWptQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Michael Paquier | 2026-08-03 22:58:31 | Re: Add pg_stat_kind_info system view |
| Previous Message | Corey Huinker | 2026-08-03 22:47:50 | Re: use of SPI by postgresImportForeignStatistics |