Re: UUID v7

On Fri, Nov 29, 2024 at 10:39 AM Andrey M. Borodin <x4mmm@yandex-team.ru> wrote:
>
>
>
> > On 29 Nov 2024, at 00:46, Peter Eisentraut <peter@eisentraut.org> wrote:
> >
> > Here as well.
>
> Peter, many thanks for the next round of review. I agree with all corrections.
> I'm sending amendments addressing your review as a separate step in patch set. Step 1 of this patch set is identical to v39.
>

Thank you for updating the patch! Here are two comments:

 <function>uuidv7</function> () <returnvalue>uuid</returnvalue>
 </synopsis>
     This function returns a version 7 UUID (UNIX timestamp with millisecond
-    precision + sub-millisecond timestamp + random).
+    precision + sub-millisecond timestamp + random). This function can accept
+    optional <parameter>shift</parameter> parameter of type
<type>interval</type>
+    which shift internal timestamp by the given interval.
   </para>

There is no "shift" parameter in the function synopsis.

Also, while reviewing the changes for func.sgml, I find that now that
we have 5 UUID functions, it might make sense to create a table for
UUID functions instead of describing functions separately. Which seems
to be more readable and consistent with other functions in docs.

---
+/*
+ * Wrapper for gen_random_uuid()
+ */
+Datum
+uuidv4(PG_FUNCTION_ARGS)
+{
+ return gen_random_uuid(fcinfo);
+}

Why do we need this? IIUC we marked uuidv4() (and uuidv7()) leafproof
because gen_random_uuid() is marked too. Otherwise, the following test
in opr_sanity would fail:

-- Considering only built-in procs (prolang = 12), look for multiple uses
-- of the same internal function (ie, matching prosrc fields).  It's OK to
-- have several entries with different pronames for the same internal function,
-- but conflicts in the number of arguments and other critical items should
-- be complained of.  (We don't check data types here; see next query.)
-- Note: ignore aggregate functions here, since they all point to the same
-- dummy built-in function.

Given that these functions don't need to be marked leakproof, does it
make sense to remove the leakproof mark from gen_random_uuid() too?
That way, we don't need the wrapper function.

Regards,

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