Re: UUID v7

On Fri, Nov 29, 2024 at 10:39 AM Andrey M. Borodin <x4mmm(at)yandex-team(dot)ru> wrote:
>
>
>
> > On 29 Nov 2024, at 00:46, Peter Eisentraut <peter(at)eisentraut(dot)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