Unmark gen_random_uuid() function leakproof
Hi,
While reviewing UUIDv7 patch[1], I realized gen_random_uuid() is
marked leakproof even though it doesn't take arguments. The functions
without arguments don't need to be marked leakproof in principle. This
is the sole function that has no arguments and is listed in the "List
of built-in leakproof functions" in opr_sanity.sql. I've attached the
patch for fixing it and for better consistency with new UUID
generation functions discussed on that thread.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
Attachments:
v1-0001-Unmark-gen_random_uuid-function-leakproof.patchapplication/octet-stream; name=v1-0001-Unmark-gen_random_uuid-function-leakproof.patchDownload
From 726ecc44991169c5b10a52e0d2bb2ad0a3ced4c0 Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Mon, 9 Dec 2024 13:53:44 -0800
Subject: [PATCH v1] Unmark gen_random_uuid() function leakproof.
The functions wihtout arguments don't need to be marked
leakproof. This commit unmarks gen_random_uuid() leakproof for
consistency with upcoming UUID generation functions.
XXX bump catalog version.
Reviewed-by:
Discussion: https://postgr.es/m/
---
src/include/catalog/pg_proc.dat | 2 +-
src/test/regress/expected/opr_sanity.out | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 9575524007f..ccf79761da5 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -9345,7 +9345,7 @@
proname => 'uuid_hash_extended', prorettype => 'int8',
proargtypes => 'uuid int8', prosrc => 'uuid_hash_extended' },
{ oid => '3432', descr => 'generate random UUID',
- proname => 'gen_random_uuid', proleakproof => 't', provolatile => 'v',
+ proname => 'gen_random_uuid', provolatile => 'v',
prorettype => 'uuid', proargtypes => '', prosrc => 'gen_random_uuid' },
{ oid => '6342', descr => 'extract timestamp from UUID',
proname => 'uuid_extract_timestamp', proleakproof => 't',
diff --git a/src/test/regress/expected/opr_sanity.out b/src/test/regress/expected/opr_sanity.out
index 34a32bd11d2..452f2572302 100644
--- a/src/test/regress/expected/opr_sanity.out
+++ b/src/test/regress/expected/opr_sanity.out
@@ -855,7 +855,6 @@ sha224(bytea)
sha256(bytea)
sha384(bytea)
sha512(bytea)
-gen_random_uuid()
starts_with(text,text)
macaddr8_eq(macaddr8,macaddr8)
macaddr8_lt(macaddr8,macaddr8)
--
2.43.5
Hi,
On 2024-12-09 14:10:30 -0800, Masahiko Sawada wrote:
While reviewing UUIDv7 patch[1], I realized gen_random_uuid() is
marked leakproof even though it doesn't take arguments. The functions
without arguments don't need to be marked leakproof in principle. This
is the sole function that has no arguments and is listed in the "List
of built-in leakproof functions" in opr_sanity.sql. I've attached the
patch for fixing it and for better consistency with new UUID
generation functions discussed on that thread.
Seems like it'd make sense to add a test to opr_sanity.sql so we don't
reintroduce such cases?
Greetings,
Andres Freund
On Mon, Dec 9, 2024 at 2:23 PM Andres Freund <andres@anarazel.de> wrote:
Hi,
On 2024-12-09 14:10:30 -0800, Masahiko Sawada wrote:
While reviewing UUIDv7 patch[1], I realized gen_random_uuid() is
marked leakproof even though it doesn't take arguments. The functions
without arguments don't need to be marked leakproof in principle. This
is the sole function that has no arguments and is listed in the "List
of built-in leakproof functions" in opr_sanity.sql. I've attached the
patch for fixing it and for better consistency with new UUID
generation functions discussed on that thread.Seems like it'd make sense to add a test to opr_sanity.sql so we don't
reintroduce such cases?
Thank you for the comment. It's a good idea. I've updated the patch.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
Attachments:
v2-0001-Unmark-gen_random_uuid-function-leakproof.patchapplication/octet-stream; name=v2-0001-Unmark-gen_random_uuid-function-leakproof.patchDownload
From 9481a6f74c3393f9143318123128f92a6827347a Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Mon, 9 Dec 2024 13:53:44 -0800
Subject: [PATCH v2] Unmark gen_random_uuid() function leakproof.
The functions without arguments don't need to be marked
leakproof. This commit unmarks gen_random_uuid() leakproof for
consistency with upcoming UUID generation functions. Also, this commit
adds a regression test to prevent reintroducing such cases.
XXX bump catalog version.
Reviewed-by: Andres Freund
Discussion: https://postgr.es/m/CAD21AoBE1ePPWY1NQEgk3DkqjYzLPZwYTzCySHm0e%2B9a69PfZw%40mail.gmail.com
---
src/include/catalog/pg_proc.dat | 2 +-
src/test/regress/expected/opr_sanity.out | 7 ++++++-
src/test/regress/sql/opr_sanity.sql | 7 +++++++
3 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 9575524007f..ccf79761da5 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -9345,7 +9345,7 @@
proname => 'uuid_hash_extended', prorettype => 'int8',
proargtypes => 'uuid int8', prosrc => 'uuid_hash_extended' },
{ oid => '3432', descr => 'generate random UUID',
- proname => 'gen_random_uuid', proleakproof => 't', provolatile => 'v',
+ proname => 'gen_random_uuid', provolatile => 'v',
prorettype => 'uuid', proargtypes => '', prosrc => 'gen_random_uuid' },
{ oid => '6342', descr => 'extract timestamp from UUID',
proname => 'uuid_extract_timestamp', proleakproof => 't',
diff --git a/src/test/regress/expected/opr_sanity.out b/src/test/regress/expected/opr_sanity.out
index 34a32bd11d2..b673642ad1d 100644
--- a/src/test/regress/expected/opr_sanity.out
+++ b/src/test/regress/expected/opr_sanity.out
@@ -855,7 +855,6 @@ sha224(bytea)
sha256(bytea)
sha384(bytea)
sha512(bytea)
-gen_random_uuid()
starts_with(text,text)
macaddr8_eq(macaddr8,macaddr8)
macaddr8_lt(macaddr8,macaddr8)
@@ -878,6 +877,12 @@ crc32(bytea)
crc32c(bytea)
bytea_larger(bytea,bytea)
bytea_smaller(bytea,bytea)
+-- Check that functions without argument are not marked as leakproof.
+SELECT p1.oid::regprocedure
+FROM pg_proc p1 JOIN pg_namespace pn
+ ON pronamespace = pn.oid
+WHERE nspname = 'pg_catalog' AND proleakproof AND pronargs = 0
+ORDER BY 1;
-- restore normal output mode
\a\t
-- List of functions used by libpq's fe-lobj.c
diff --git a/src/test/regress/sql/opr_sanity.sql b/src/test/regress/sql/opr_sanity.sql
index 2fe7b6dcc49..2fb3a852878 100644
--- a/src/test/regress/sql/opr_sanity.sql
+++ b/src/test/regress/sql/opr_sanity.sql
@@ -399,6 +399,13 @@ FROM pg_proc p1 JOIN pg_namespace pn
WHERE nspname = 'pg_catalog' AND proleakproof
ORDER BY 1;
+-- Check that functions without argument are not marked as leakproof.
+SELECT p1.oid::regprocedure
+FROM pg_proc p1 JOIN pg_namespace pn
+ ON pronamespace = pn.oid
+WHERE nspname = 'pg_catalog' AND proleakproof AND pronargs = 0
+ORDER BY 1;
+
-- restore normal output mode
\a\t
--
2.43.5
On Mon, Dec 9, 2024 at 2:48 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
On Mon, Dec 9, 2024 at 2:23 PM Andres Freund <andres@anarazel.de> wrote:
Hi,
On 2024-12-09 14:10:30 -0800, Masahiko Sawada wrote:
While reviewing UUIDv7 patch[1], I realized gen_random_uuid() is
marked leakproof even though it doesn't take arguments. The functions
without arguments don't need to be marked leakproof in principle. This
is the sole function that has no arguments and is listed in the "List
of built-in leakproof functions" in opr_sanity.sql. I've attached the
patch for fixing it and for better consistency with new UUID
generation functions discussed on that thread.Seems like it'd make sense to add a test to opr_sanity.sql so we don't
reintroduce such cases?Thank you for the comment. It's a good idea. I've updated the patch.
I'm going to push the updated patch tomorrow, barring objections and
further comments.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
On Tue, Dec 10, 2024 at 1:52 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
On Mon, Dec 9, 2024 at 2:48 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
On Mon, Dec 9, 2024 at 2:23 PM Andres Freund <andres@anarazel.de> wrote:
Hi,
On 2024-12-09 14:10:30 -0800, Masahiko Sawada wrote:
While reviewing UUIDv7 patch[1], I realized gen_random_uuid() is
marked leakproof even though it doesn't take arguments. The functions
without arguments don't need to be marked leakproof in principle. This
is the sole function that has no arguments and is listed in the "List
of built-in leakproof functions" in opr_sanity.sql. I've attached the
patch for fixing it and for better consistency with new UUID
generation functions discussed on that thread.Seems like it'd make sense to add a test to opr_sanity.sql so we don't
reintroduce such cases?Thank you for the comment. It's a good idea. I've updated the patch.
I'm going to push the updated patch tomorrow, barring objections and
further comments.
Pushed (398d3e3b5b).
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com