public inbox for [email protected]
help / color / mirror / Atom feedFrom: Masahiko Sawada <[email protected]>
To: Andrey Borodin <[email protected]>
Cc: Daniel Verite <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Jelte Fennema-Nio <[email protected]>
Cc: Sergey Prokhorenko <[email protected]>
Cc: Przemysław Sztoch <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Aleksander Alekseev <[email protected]>
Cc: Pgsql-Hackers Mailing List <[email protected]>
Cc: David G. Johnston <[email protected]>
Cc: Mat Arye <[email protected]>
Cc: Matthias van de Meent <[email protected]>
Cc: Nikolay Samokhvalov <[email protected]>
Cc: Junwang Zhao <[email protected]>
Cc: Stepan Neretin <[email protected]>
Subject: Re: UUID v7
Date: Tue, 25 Mar 2025 21:32:49 -0700
Message-ID: <CAD21AoAVU-VnuqvdafqUCHCAo7Msb6_2k7nSzd_fijMbbtKrCg@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<CAD21AoBhtGCR0ZnaYhPHBJg7jzxM5Sco3ZuHCvP45xevcPRqbw@mail.gmail.com>
<[email protected]>
<CAD21AoAJ5ETmSUgm9OL7qR_7Ogm9FPsVn+AwkA4Dr4JrrDQPEg@mail.gmail.com>
<[email protected]>
<CAD21AoCkn2GCsDePwkczFwf-=WR3+DGGnb+pTZkkgYgit453fw@mail.gmail.com>
<[email protected]>
<CAD21AoB5Fo6YcPpLwyYRw1RG85iQ8Lj=qnrip1c6jUpStfK+qw@mail.gmail.com>
<[email protected]>
<CAD21AoDfHfF8=h5vauLOmVSxS6dnNGHmqT4h0gSoj1jM-qRjnQ@mail.gmail.com>
<[email protected]>
<CAD21AoAWyPYhqyP1BmKBy7f+UG8hOntLntauxgxLar0rHGihSQ@mail.gmail.com>
<[email protected]>
On Sun, Feb 9, 2025 at 9:07 AM Andrey Borodin <[email protected]> wrote:
>
> I've took into account note from Sergey that "offset" is better name for uuidv7() argument than "shift".
>
> > On 5 Feb 2025, at 03:02, Masahiko Sawada <[email protected]> wrote:
> >
> >>
> >> I was thinking about incorporating test like this.
> >>
> >>>> With this patch we can generate correct UUIDs in a very distant future.
> >>>> postgres=# select x, uuid_extract_timestamp(uuidv7((x::text || ' year'::text)::interval)),
> >>>> (x::text || ' year'::text)::interval
> >>>> from generate_series(1,9000,1000) x;
> >>>> x | uuid_extract_timestamp | interval
> >>>> ------+-----------------------------+------------
> >>>> 1 | 2026-01-31 12:00:53.084+05 | 1 year
> >>>> 1001 | 3026-01-31 12:00:53.084+05 | 1001 years
> >>>> 2001 | 4026-01-31 12:00:53.084+05 | 2001 years
> >>>> 3001 | 5026-01-31 12:00:53.084+05 | 3001 years
> >>>> 4001 | 6026-01-31 12:00:53.084+05 | 4001 years
> >>>> 5001 | 7026-01-31 12:00:53.085+05 | 5001 years
> >>>> 6001 | 8026-01-31 12:00:53.085+05 | 6001 years
> >>>> 7001 | 9026-01-31 12:00:53.085+05 | 7001 years
> >>>> 8001 | 10026-01-31 12:00:53.085+05 | 8001 years
> >>>> (9 rows)
> >>
> >
> > Something like following queries might be workable for example?
> >
> > create table test (c serial, d uuid, t timestamptz generated always as
> > (uuid_extract_timestamp(d)) stored);
> > insert into test (d) select uuidv7((n || 'years')::interval) from
> > generate_series(1, 2000) n;
> > select count(*) from (select t - lag(t) over (order by c) as diff from
> > test) where diff > '10 year' ;
>
> Yeah, makes sense. I reduced tolerance to 366+1 day. Must be stable if we've done all the time offset business right.
>
> > Here are some review comments:
> >
> > #define NS_PER_S INT64CONST(1000000000)
> > #define NS_PER_MS INT64CONST(1000000)
> > +#define US_PER_MS INT64CONST(1000)
> > #define NS_PER_US INT64CONST(1000)
> >
> > I think it's clear if we put US_PER_MS below NS_PER_US.
>
> OK.
>
> > ---
> > *
> > - * ns is a number of nanoseconds since start of the UNIX epoch. This value is
> > + * unix_ts_ms is a number of milliseconds since start of the UNIX epoch,
> > + * ns_in_ms is a number of nanoseconds within millisecond. These values are
> > * used for time-dependent bits of UUID.
> >
> > I think we can mention that the RFC describes that stored unix
> > timestamp as an unsigned integer.
>
> Done. Feel free to adjust my wordings, I've no sense of idiomatic English.
>
> >
> > ---
> > static pg_uuid_t *
> > -generate_uuidv7(int64 ns)
> > +generate_uuidv7(uint64 unix_ts_ms, uint32 ns_in_ms)
> >
> > How about renaming ns_in_ms with sub_ms?
>
> OK.
>
> >
> > ---
> > + /* 64 bits is enough for real time, but not for a time range of UUID */
> >
> > I could not understand the point of this comment. It seems to say that
> > 64-bits is not enough for a time range of UUID, but doesn't the time
> > range of UUIDv7 use only 48 bits? It seems to need more comments.
>
> I've tried to say that acquiring current time as an int64 ns since UNIX epoch is still viable for the code (until year 2262).
>
>
> > ---
> > - ns = (ts + (POSTGRES_EPOCH_JDATE - UNIX_EPOCH_JDATE) *
> > SECS_PER_DAY * USECS_PER_SEC)
> > - * NS_PER_US + ns % NS_PER_US;
> > + us = (ts + (POSTGRES_EPOCH_JDATE - UNIX_EPOCH_JDATE) *
> > SECS_PER_DAY * USECS_PER_SEC);
> >
> > /* Generate an UUIDv7 */
> > - uuid = generate_uuidv7(ns);
> > + uuid = generate_uuidv7(us / US_PER_MS, (us % US_PER_MS) *
> > NS_PER_US + ns % NS_PER_US);
> >
> > Need to update comments in uuidv7_internval() such as:
> >
> > /*
> > * Shift the current timestamp by the given interval. To calculate time
> > * shift correctly, we convert the UNIX epoch to TimestampTz and use
> > * timestamptz_pl_interval(). Since this calculation is done with
> > * microsecond precision, we carry nanoseconds from original ns value to
> > * shifted ns value.
> > */
> >
> > and
> >
> > /*
> > * Convert a TimestampTz value back to an UNIX epoch and back nanoseconds.
> > */
>
> I've tried. I'm not very satisfied with comments, but could not come up with easier description.
>
Thank you for updating the patch. I had missed to track this patch.
I've updated the patch from your v4 patch. In this version, I excluded
the argument name change (from 'shift' to 'offset') as it's not
related to the bug fix and simplified the regression test case.
Please review it.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
Attachments:
[application/octet-stream] v5-0001-Fix-timestamp-overflow-in-UUIDv7-implementation.patch (6.9K, ../CAD21AoAVU-VnuqvdafqUCHCAo7Msb6_2k7nSzd_fijMbbtKrCg@mail.gmail.com/2-v5-0001-Fix-timestamp-overflow-in-UUIDv7-implementation.patch)
download | inline diff:
From 82ba268b7af93a75c76cf36a85c764761e0dbeb1 Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <[email protected]>
Date: Tue, 25 Mar 2025 15:14:42 -0700
Subject: [PATCH v5] Fix timestamp overflow in UUIDv7 implementation.
Previously, the uuidv7_interval() function performed timestamp
shifting calculations using microsecond precision, but then converted
the result back to nanosecond precision. Since the millisecond and
sub-millisecond parts were extracted from this nanosecond timestamp
and stored into the UUIDv7 value, overflow occurred for timestamps
beyond the year 2262.
With this commit, the millisecond and sub-millisecond parts are stored
directly into the UUIDv7 value without being converted back to a
nanosecond precision timestamp. Following RFC 9562, the timestamp is
stored as an unsigned integer, enabling support for dates up to the
year 10889.
Reported and fixed by Andrey Borodin, with cosmetic changes and
regression tests by me.
Reported-by: Andrey Borodin <[email protected]>
Author: Andrey Borodin <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
src/backend/utils/adt/uuid.c | 34 +++++++++++++++---------------
src/test/regress/expected/uuid.out | 14 ++++++++++++
src/test/regress/sql/uuid.sql | 11 ++++++++++
3 files changed, 42 insertions(+), 17 deletions(-)
diff --git a/src/backend/utils/adt/uuid.c b/src/backend/utils/adt/uuid.c
index 4f8402ef925..be0f0f9f1ce 100644
--- a/src/backend/utils/adt/uuid.c
+++ b/src/backend/utils/adt/uuid.c
@@ -29,6 +29,7 @@
#define NS_PER_S INT64CONST(1000000000)
#define NS_PER_MS INT64CONST(1000000)
#define NS_PER_US INT64CONST(1000)
+#define US_PER_MS INT64CONST(1000)
/*
* UUID version 7 uses 12 bits in "rand_a" to store 1/4096 (or 2^12) fractions of
@@ -69,6 +70,7 @@ static bool uuid_abbrev_abort(int memtupcount, SortSupport ssup);
static Datum uuid_abbrev_convert(Datum original, SortSupport ssup);
static inline void uuid_set_version(pg_uuid_t *uuid, unsigned char version);
static inline int64 get_real_time_ns_ascending();
+static pg_uuid_t *generate_uuidv7(uint64 unix_ts_ms, uint32 sub_ms);
Datum
uuid_in(PG_FUNCTION_ARGS)
@@ -523,17 +525,17 @@ get_real_time_ns_ascending()
* described in the RFC. This method utilizes 12 bits from the "rand_a" bits
* to store a 1/4096 (or 2^12) fraction of sub-millisecond precision.
*
- * ns is a number of nanoseconds since start of the UNIX epoch. This value is
+ * unix_ts_ms is a number of milliseconds since start of the UNIX epoch,
+ * and sub_ms is a number of nanoseconds within millisecond. These values are
* used for time-dependent bits of UUID.
+ *
+ * NB: all numbers here are unsigned, unix_ts_ms cannot be negative per RFC.
*/
static pg_uuid_t *
-generate_uuidv7(int64 ns)
+generate_uuidv7(uint64 unix_ts_ms, uint32 sub_ms)
{
pg_uuid_t *uuid = palloc(UUID_LEN);
- int64 unix_ts_ms;
- int32 increased_clock_precision;
-
- unix_ts_ms = ns / NS_PER_MS;
+ uint32 increased_clock_precision;
/* Fill in time part */
uuid->data[0] = (unsigned char) (unix_ts_ms >> 40);
@@ -547,7 +549,7 @@ generate_uuidv7(int64 ns)
* sub-millisecond timestamp fraction (SUBMS_BITS bits, not
* SUBMS_MINIMAL_STEP_BITS)
*/
- increased_clock_precision = ((ns % NS_PER_MS) * (1 << SUBMS_BITS)) / NS_PER_MS;
+ increased_clock_precision = (sub_ms * (1 << SUBMS_BITS)) / NS_PER_MS;
/* Fill the increased clock precision to "rand_a" bits */
uuid->data[6] = (unsigned char) (increased_clock_precision >> 8);
@@ -586,7 +588,8 @@ generate_uuidv7(int64 ns)
Datum
uuidv7(PG_FUNCTION_ARGS)
{
- pg_uuid_t *uuid = generate_uuidv7(get_real_time_ns_ascending());
+ int64 ns = get_real_time_ns_ascending();
+ pg_uuid_t *uuid = generate_uuidv7(ns / NS_PER_MS, ns % NS_PER_MS);
PG_RETURN_UUID_P(uuid);
}
@@ -601,13 +604,13 @@ uuidv7_interval(PG_FUNCTION_ARGS)
TimestampTz ts;
pg_uuid_t *uuid;
int64 ns = get_real_time_ns_ascending();
+ int64 us;
/*
* Shift the current timestamp by the given interval. To calculate time
* shift correctly, we convert the UNIX epoch to TimestampTz and use
- * timestamptz_pl_interval(). Since this calculation is done with
- * microsecond precision, we carry nanoseconds from original ns value to
- * shifted ns value.
+ * timestamptz_pl_interval(). This calculation is done with microsecond
+ * precision.
*/
ts = (TimestampTz) (ns / NS_PER_US) -
@@ -618,14 +621,11 @@ uuidv7_interval(PG_FUNCTION_ARGS)
TimestampTzGetDatum(ts),
IntervalPGetDatum(shift)));
- /*
- * Convert a TimestampTz value back to an UNIX epoch and back nanoseconds.
- */
- ns = (ts + (POSTGRES_EPOCH_JDATE - UNIX_EPOCH_JDATE) * SECS_PER_DAY * USECS_PER_SEC)
- * NS_PER_US + ns % NS_PER_US;
+ /* Convert a TimestampTz value back to an UNIX epoch timestamp */
+ us = ts + (POSTGRES_EPOCH_JDATE - UNIX_EPOCH_JDATE) * SECS_PER_DAY * USECS_PER_SEC;
/* Generate an UUIDv7 */
- uuid = generate_uuidv7(ns);
+ uuid = generate_uuidv7(us / US_PER_MS, (us % US_PER_MS) * NS_PER_US + ns % NS_PER_US);
PG_RETURN_UUID_P(uuid);
}
diff --git a/src/test/regress/expected/uuid.out b/src/test/regress/expected/uuid.out
index 798633ad51e..cbd497376c4 100644
--- a/src/test/regress/expected/uuid.out
+++ b/src/test/regress/expected/uuid.out
@@ -233,6 +233,20 @@ SELECT array_agg(id ORDER BY guid_field) FROM guid3;
{1,2,3,4,5,6,7,8,9,10}
(1 row)
+-- Check the timestamp offsets for v7.
+--
+-- generate UUIDv7 having timestamps up to 10889 year, which is the maximum year
+-- can be stored in UUIDv7, and then check if the timestamps extracted from UUIDv7
+-- values are not overflowed.
+WITH uuidts AS (
+ SELECT y, ts as ts, lag(ts) OVER (ORDER BY y) AS prev_ts
+ FROM (SELECT y, uuid_extract_timestamp(uuidv7((y || ' years')::interval)) AS ts FROM generate_series(-50, 10889 - extract(year from now())::int) y)
+)
+SELECT y, ts, prev_ts FROM uuidts WHERE ts < prev_ts;
+ y | ts | prev_ts
+---+----+---------
+(0 rows)
+
-- extract functions
-- version
SELECT uuid_extract_version('11111111-1111-5111-8111-111111111111'); -- 5
diff --git a/src/test/regress/sql/uuid.sql b/src/test/regress/sql/uuid.sql
index 110188361d1..cd0e65d3a8b 100644
--- a/src/test/regress/sql/uuid.sql
+++ b/src/test/regress/sql/uuid.sql
@@ -119,6 +119,17 @@ SELECT count(DISTINCT guid_field) FROM guid1;
INSERT INTO guid3 (guid_field) SELECT uuidv7() FROM generate_series(1, 10);
SELECT array_agg(id ORDER BY guid_field) FROM guid3;
+-- Check the timestamp offsets for v7.
+--
+-- generate UUIDv7 having timestamps up to 10889 year, which is the maximum year
+-- can be stored in UUIDv7, and then check if the timestamps extracted from UUIDv7
+-- values are not overflowed.
+WITH uuidts AS (
+ SELECT y, ts as ts, lag(ts) OVER (ORDER BY y) AS prev_ts
+ FROM (SELECT y, uuid_extract_timestamp(uuidv7((y || ' years')::interval)) AS ts FROM generate_series(-50, 10889 - extract(year from now())::int) y)
+)
+SELECT y, ts, prev_ts FROM uuidts WHERE ts < prev_ts;
+
-- extract functions
-- version
--
2.43.5
view thread (68+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: UUID v7
In-Reply-To: <CAD21AoAVU-VnuqvdafqUCHCAo7Msb6_2k7nSzd_fijMbbtKrCg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox