On 27.11.24 00:11, Masahiko Sawada wrote:
On Tue, Nov 26, 2024 at 1:55 PM Jelte Fennema-Nio <postg...@jeltef.nl> wrote:
On Tue, 26 Nov 2024 at 21:48, Sergey Prokhorenko
<sergeyprokhore...@yahoo.com.au> wrote:
gen_uuidv7() is OK
I'd very much prefer to not have a gen_ or get_ prefix as argued before[1][2].
My vote is still for simply uuidv7() and uuidv4()
uuid-ossp is outdated, slow and not supported by the author. UUIDv7 is the
renaissance of UUIDs. So we should not depend on legacy technology names
agreed
It seems that we agreed to use 'uuidv7' instead of 'uuid_v7()'. There
is discussion whether we should add 'gen_' or 'get_' but let's go back
to the previously-agreed function name 'uuidv7()' for now. We can
rename it later if we find a better name.
I've attached the new version patch that incorporated all comments and
renamed the functions. Also I avoided using 'if defined(__darwin__) ||
defined(_MSC_VER)' twice.
* doc/src/sgml/func.sgml
The function variant uuidv7(interval) is not documented.
* src/backend/utils/adt/uuid.c
+/* Set the given UUID version and the variant bits */
+static inline void
+uuid_set_version(pg_uuid_t *uuid, unsigned char version)
+{
This should be a block comment, like
/*
* Set the ...
*/
+/*
+ * Generate UUID version 7 per RFC 9562, with the given timestamp.
+ *
...
+static pg_attribute_always_inline pg_uuid_t *
+generate_uuidv7(int64 ns)
Is "ns" the timestamp argument? What format is it? Explain.
+ /*
+ * Shift the current timestamp by the given interval. To make correct
+ * calculating the time shift, we convert the UNIX epoch to TimestampTz
+ * and use timestamptz_pl_interval(). Since this calculation is done
with
+ * microsecond precision, we carry back the nanoseconds.
+ */
This needs a bit of grammar tweaking, I think: "To make correct calculating"
I don't know what the meaning of "carry back" is.
+ Interval *span = PG_GETARG_INTERVAL_P(0);
Not sure why this is named "span"? Maybe "shift" would be better?
* src/include/catalog/pg_proc.dat
+{ oid => '9897', descr => 'generate UUID version 7 with a timestamp
shifted on specific interval',
better "shifted by"?
* src/test/regress/expected/opr_sanity.out
+uuidv4()
+uuidv7()
+uuidv7(interval)
Functions without arguments don't need to be marked leakproof.
uuidv7(interval) internally calls timestamptz_pl_interval(), which is
not leakproof, so I don't think that classification is sound.
* src/test/regress/sql/uuid.sql
+SELECT uuid_extract_version(uuidv4()); --4
+SELECT uuid_extract_version(uuidv7()); --7
Make the whitespace of the comment consistent with the rest of the file.
-SELECT uuid_extract_timestamp('C232AB00-9414-11EC-B3C8-9F6BDECED846') =
'Tuesday, February 22, 2022 2:22:22.00 PM GMT+05:00'; -- RFC 4122bis
test vector
+SELECT uuid_extract_timestamp('C232AB00-9414-11EC-B3C8-9F6BDECED846') =
'Tuesday, February 22, 2022 2:22:22.00 PM GMT+05:00'; -- RFC 9562 test
vector for v1
Here as well.
