Re: UUID v7 - Mailing list pgsql-hackers

On Mon, 25 Nov 2024 at 12:11, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> On Mon, Nov 25, 2024 at 10:15 AM Andrey M. Borodin <x4mmm@yandex-team.ru> wrote:
>>
>>
>>
>> > On 25 Nov 2024, at 22:53, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>> >
>> > In the following code, we use "defined(__darwin__) || defined(_MSC_VER)":
>> >
>> > +#if defined(__darwin__) || defined(_MSC_VER)
>> > +#define SUBMS_MINIMAL_STEP_BITS 10
>> > +#else
>> > +#define SUBMS_MINIMAL_STEP_BITS 12
>> > +#endif
>> > #define SUBMS_BITS     12
>> > -#define SUBMS_MINIMAL_STEP_NS ((NS_PER_MS / (1 << SUBMS_BITS)) + 1)
>> > +#define SUBMS_MINIMAL_STEP_NS ((NS_PER_MS / (1 <<
>> > SUBMS_MINIMAL_STEP_BITS)) + 1)
>> >
>> > on the other hand, we use "defined(__darwin__) || defined(WIN32)" here:
>> >
>> > +#if defined(__darwin__) || defined(WIN32)
>> > +       /*
>> > +        * On MacOS real time is truncted to microseconds. Thus, 2 least
>> > +        * significant are dependent on other time-specific bits, thus
>> > they do not
>> > +        * contribute to uniqueness. To make these bit random we mix in two bits
>> > +        * from CSPRNG.
>> > +        *
>> > +        * SUBMS_MINIMAL_STEP is chosen so that we still guarantee monotonicity
>> > +        * despite altering these bits.
>> > +        */
>> > +       uuid->data[7] = uuid->data[7] ^ (uuid->data[8] >> 6);
>> > +#endif
>> >
>> > Is there a reason for using different macros?
>>
>> No, that's an oversight. We should mix these 2 bits if an only if SUBMS_MINIMAL_STEP_BITS=10.
>>
>> <tldr>
>> In your review change_v33.patch you used WIN32, but it did not actually compile on Windows.
>> So on Saturday I squashed v33+change_v33.patch, and composed a
>> message that I think we still should switch to _MSC_VER. And just
>> before sending I received your message with v36 where you used
>> _MSC_VER :)
>>
>> I think this way:
>> _MSC_VER - native Windows without clock_gettime, we used gettimeofday() and 10 bits of sub-ms.
>> MinGW - we use clock_gettime() and 12 bits.
>> Darwin - we use clock_gettime() and 10 bits.
>> Anything else -  clock_gettime() and 12 bits.
>> </tldr>
>
> Thank you for the summary.
>
> On MinGW, IIUC we can get 100-ns precision timestamps[1], so using 12
> bits for calculating the minimal step would make sense.
>
> Also, if we implement the Windows port of clock_gettime() in the
> future, we can remove the part of using gettimeofday() in
> get_real_time_ns_ascending(). It seems to me that it's
> over-engineering to implement that part only for the UUID v7. So the
> current implementation of get_real_time_ns_ascending() makes sense to
> me.
>
>>
>> >
>> > In get_real_time_ns_ascending(), we use _MSC_VER so we use
>> > clock_gettime() on MinGW.
>> >
>> >>
>> >> Sergey Prokhorenko just draw my attention to the new release of MariaDB [0]. They are doing very similar UUID v7
generationas we do [1]. 
>> >>
>> >
>> > Thank you for the references. It made me think that we can use the
>> > function name uuid_v7() rather than uuidv7().
>>
>> I think it's a good idea if we will be kind of SQL-compatible.
>>
>
> Okay, let"s rename it.
>
> I've merged patches and renamed functions (also updated the commit
> message). Please find the attachment.
>

It seems a typo about uuid_v{4,7}.

+<function>uuidv_4</function> () <returnvalue>uuid</returnvalue>
+</synopsis>
+   These functions return a version 4 (random) UUID.
+<synopsis>
+<function>uuidv_7</function> () <returnvalue>uuid</returnvalue>

--
Regrads,
Japin Li