Fix for typo in UUIDv7 timestamp extraction

Started by Erik Nordström5 months ago5 messages
#1Erik Nordström
erik@tigerdata.com
1 attachment(s)

Hi,

I think I found a small typo in the function that extracts a timestamp from
a UUIDv7 (uuid_extract_timestamp). Unless I am mistaken, the constant
US_PER_MS should be used instead of NS_PER_US when converting milliseconds
to microseconds. Fortunately, these constants are the same so the
calculation is still correct.

Anyway, attaching a patch to fix this typo.

On a related note, I am wondering why this function doesn't extract and use
the sub-millisecond information in the rand_a bits? These bits are added
when generating the UUID, but they don't seem to be extracted. Hopefully
somebody could shed some light on this and whether it would be a worthwhile
addition.

Thank you,

- Erik

Attachments:

0001-Fix-constant-when-extracting-timestamp-from-UUIDv7.patchtext/x-patch; charset=US-ASCII; name=0001-Fix-constant-when-extracting-timestamp-from-UUIDv7.patchDownload
From 5c8b4070366581219bab0cdda27336bb0cea5844 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Erik=20Nordstr=C3=B6m?= <erik.nordstrom@gmail.com>
Date: Wed, 13 Aug 2025 09:41:08 +0200
Subject: [PATCH] Fix constant when extracting timestamp from UUIDv7

When extracting a timestamp from a UUIDv7, a conversion from
milliseconds to microseconds is performed using the wrong constant
NS_PER_US instead of US_PER_MS. Fortunately, these constants are both
the same so the fix is mostly to avoid confusion.
---
 src/backend/utils/adt/uuid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/backend/utils/adt/uuid.c b/src/backend/utils/adt/uuid.c
index bce7309c183..a746388b073 100644
--- a/src/backend/utils/adt/uuid.c
+++ b/src/backend/utils/adt/uuid.c
@@ -752,7 +752,7 @@ uuid_extract_timestamp(PG_FUNCTION_ARGS)
 			+ (((uint64) uuid->data[0]) << 40);
 
 		/* convert ms to us, then adjust */
-		ts = (TimestampTz) (tms * NS_PER_US) -
+		ts = (TimestampTz) (tms * US_PER_MS) -
 			(POSTGRES_EPOCH_JDATE - UNIX_EPOCH_JDATE) * SECS_PER_DAY * USECS_PER_SEC;
 
 		PG_RETURN_TIMESTAMPTZ(ts);
-- 
2.48.1

#2Andrey Borodin
x4mmm@yandex-team.ru
In reply to: Erik Nordström (#1)
Re: Fix for typo in UUIDv7 timestamp extraction

Hi Erik!

On 13 Aug 2025, at 11:05, Erik Nordström <erik@tigerdata.com> wrote:

I think I found a small typo in the function that extracts a timestamp from a UUIDv7 (uuid_extract_timestamp). Unless I am mistaken, the constant US_PER_MS should be used instead of NS_PER_US when converting milliseconds to microseconds. Fortunately, these constants are the same so the calculation is still correct.

Wow, that's a very good level of proofreading! Yes, you are correct, it's must be US_PER_MS.

Anyway, attaching a patch to fix this typo.

LGTM.

On a related note, I am wondering why this function doesn't extract and use the sub-millisecond information in the rand_a bits? These bits are added when generating the UUID, but they don't seem to be extracted. Hopefully somebody could shed some light on this and whether it would be a worthwhile addition.

UUID might be formed by any external system, rand_a bits are not guaranteed to be non-random. Well, in some sense, reading time is reading random number, but anyway, we don't know for sure how those bits are used. And even in Postgres usage of sub-millisecond fractions depends on OS.

I think we can have a function in an extensions, that does pretend that UUID was generated by known algorithm. Or even SQL function to extract microseconds.

Thanks!

Best regards, Andrey Borodin.

#3Erik Nordström
erik@tigerdata.com
In reply to: Andrey Borodin (#2)
Re: Fix for typo in UUIDv7 timestamp extraction

On Wed, Aug 13, 2025 at 11:52 AM Andrey Borodin <x4mmm@yandex-team.ru>
wrote:

Hi Erik!

On 13 Aug 2025, at 11:05, Erik Nordström <erik@tigerdata.com> wrote:

I think I found a small typo in the function that extracts a timestamp

from a UUIDv7 (uuid_extract_timestamp). Unless I am mistaken, the constant
US_PER_MS should be used instead of NS_PER_US when converting milliseconds
to microseconds. Fortunately, these constants are the same so the
calculation is still correct.

Wow, that's a very good level of proofreading! Yes, you are correct, it's
must be US_PER_MS.

Anyway, attaching a patch to fix this typo.

LGTM.

Ok, maybe a committer would like to commit this change immediately?

On a related note, I am wondering why this function doesn't extract and

use the sub-millisecond information in the rand_a bits? These bits are
added when generating the UUID, but they don't seem to be extracted.
Hopefully somebody could shed some light on this and whether it would be a
worthwhile addition.

UUID might be formed by any external system, rand_a bits are not
guaranteed to be non-random. Well, in some sense, reading time is reading
random number, but anyway, we don't know for sure how those bits are used.
And even in Postgres usage of sub-millisecond fractions depends on OS.

I think we can have a function in an extensions, that does pretend that
UUID was generated by known algorithm. Or even SQL function to extract
microseconds.

That's what I suspected. Thanks for clarifying.

Best,

Erik

#4Masahiko Sawada
sawada.mshk@gmail.com
In reply to: Erik Nordström (#3)
Re: Fix for typo in UUIDv7 timestamp extraction

On Thu, Aug 14, 2025 at 5:10 AM Erik Nordström <erik@tigerdata.com> wrote:

On Wed, Aug 13, 2025 at 11:52 AM Andrey Borodin <x4mmm@yandex-team.ru> wrote:

Hi Erik!

On 13 Aug 2025, at 11:05, Erik Nordström <erik@tigerdata.com> wrote:

I think I found a small typo in the function that extracts a timestamp from a UUIDv7 (uuid_extract_timestamp). Unless I am mistaken, the constant US_PER_MS should be used instead of NS_PER_US when converting milliseconds to microseconds. Fortunately, these constants are the same so the calculation is still correct.

Wow, that's a very good level of proofreading! Yes, you are correct, it's must be US_PER_MS.

Anyway, attaching a patch to fix this typo.

LGTM.

Ok, maybe a committer would like to commit this change immediately?

Thank you for the report! I'll push the patch to master and v18 shortly.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

#5Masahiko Sawada
sawada.mshk@gmail.com
In reply to: Masahiko Sawada (#4)
Re: Fix for typo in UUIDv7 timestamp extraction

On Fri, Aug 15, 2025 at 11:44 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

On Thu, Aug 14, 2025 at 5:10 AM Erik Nordström <erik@tigerdata.com> wrote:

On Wed, Aug 13, 2025 at 11:52 AM Andrey Borodin <x4mmm@yandex-team.ru> wrote:

Hi Erik!

On 13 Aug 2025, at 11:05, Erik Nordström <erik@tigerdata.com> wrote:

I think I found a small typo in the function that extracts a timestamp from a UUIDv7 (uuid_extract_timestamp). Unless I am mistaken, the constant US_PER_MS should be used instead of NS_PER_US when converting milliseconds to microseconds. Fortunately, these constants are the same so the calculation is still correct.

Wow, that's a very good level of proofreading! Yes, you are correct, it's must be US_PER_MS.

Anyway, attaching a patch to fix this typo.

LGTM.

Ok, maybe a committer would like to commit this change immediately?

Thank you for the report! I'll push the patch to master and v18 shortly.

Pushed.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com