On Fri, Nov 29, 2024 at 10:39 AM Andrey M. Borodin <x4...@yandex-team.ru> wrote: > > > > > On 29 Nov 2024, at 00:46, Peter Eisentraut <pe...@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