[PATCH] Add pg_get_role_ddl() functions for role recreation

On 10/25/25 4:03 AM, Bryan Green wrote:

Attached is a patch adding two new functions for generating DDL to
recreate roles: pg_get_role_ddl() and pg_get_role_ddl_statements().

It is no longer apply to the latest code. Could you rebase this?

On 11/6/2025 1:20 AM, Quan Zongliang wrote:

On 10/25/25 4:03 AM, Bryan Green wrote:

Attached is a patch adding two new functions for generating DDL to
recreate roles: pg_get_role_ddl() and pg_get_role_ddl_statements().

It is no longer apply to the latest code. Could you rebase this?

These functions return the CREATE ROLE statement and any ALTER ROLE SET
configuration parameters needed to recreate a role.  The former returns
everything as a single text string, while the latter returns each
statement as a separate row for easier programmatic processing.

The main use case is dumping role definitions for migration or backup
purposes without needing pg_dumpall.  The functions handle all role
attributes (LOGIN, SUPERUSER, etc.) and both role-wide and
database-specific configuration parameters.

We intentionally don't include passwords, since we can only see the
hashed values.  System roles (names starting with "pg_") are rejected
with an error, as users shouldn't be recreating those anyway.

To test:

   CREATE ROLE testrole LOGIN CREATEDB CONNECTION LIMIT 5;
   ALTER ROLE testrole SET work_mem TO '64MB';
   SELECT pg_get_role_ddl('testrole');

Should produce:

   CREATE ROLE testrole LOGIN NOSUPERUSER CREATEDB NOCREATEROLE
INHERIT NOREPLICATION NOBYPASSRLS CONNECTION LIMIT 5;
   ALTER ROLE testrole SET work_mem TO '64MB';

The patch includes regression tests covering various role configurations.

Co-authored-by: Mario Gonzalez and Bryan Green.

Comments?

BG

The rebased patch is attached.

Thanks,

--
Bryan Green
EDB: https://www.enterprisedb.com

On 11/7/25 12:20 AM, Bryan Green wrote:

The rebased patch is attached.

Thanks,

Currently, we have the "CREATE USER" command. Should we consider
outputting users with the "LOGIN" attribute as "CREATE USER"?
Otherwise, it might look a little strange.

postgres=# CREATE USER testuser;
CREATE ROLE
postgres=# SELECT pg_get_role_ddl('testuser');
pg_get_role_ddl
---------------------------------------------------------------------------------------------------
CREATE ROLE testuser LOGIN NOSUPERUSER NOCREATEDB NOCREATEROLE INHERIT
NOREPLICATION NOBYPASSRLS;
(1 row)

The drawback of doing this is that the command "CREATE ROLE xxx LOGIN"
will be converted to "CREATE USER". But I still think this is better.

Regards,

--
Quan Zongliang

Hello Bryan,

I reviewed your patch and found one potential issue, please check it.
In pg_get_role_ddl_internal, the variable rolname is assigned from NameStr(roleform->rolname) (line 588), which means it points directly into the tuple returned from pg_authid. After constructing the initial CREATE ROLE statement, the code calls ReleaseSysCache(tuple); (line 665), so the memory holding that NameData now belongs to the cache again. However, the function continues to use rolname when building the subsequent ALTER ROLE statements (lines 756–765). Because the tuple has already been released, rolname is a dangling pointer and we risk reading garbage or crashing later. To fix this, copy the role name before releasing the syscache, e.g. rolname = pstrdup(NameStr(roleform->rolname));, and free it at the end.

BR,
Yuan Li (Carol)

-----邮件原件-----
发件人: Bryan Green <dbryan.green@gmail.com>
发送时间: 2025年11月7日 0:21
收件人: Quan Zongliang <quanzongliang@yeah.net>; pgsql-hackers@lists.postgresql.org
主题: Re: [PATCH] Add pg_get_role_ddl() functions for role recreation

On 11/6/2025 1:20 AM, Quan Zongliang wrote:

On 10/25/25 4:03 AM, Bryan Green wrote:

Attached is a patch adding two new functions for generating DDL to
recreate roles: pg_get_role_ddl() and pg_get_role_ddl_statements().

It is no longer apply to the latest code. Could you rebase this?

These functions return the CREATE ROLE statement and any ALTER ROLE
SET configuration parameters needed to recreate a role.  The former
returns everything as a single text string, while the latter returns
each statement as a separate row for easier programmatic processing.

The main use case is dumping role definitions for migration or backup
purposes without needing pg_dumpall.  The functions handle all role
attributes (LOGIN, SUPERUSER, etc.) and both role-wide and
database-specific configuration parameters.

We intentionally don't include passwords, since we can only see the
hashed values.  System roles (names starting with "pg_") are rejected
with an error, as users shouldn't be recreating those anyway.

To test:

   CREATE ROLE testrole LOGIN CREATEDB CONNECTION LIMIT 5;
   ALTER ROLE testrole SET work_mem TO '64MB';
   SELECT pg_get_role_ddl('testrole');

Should produce:

   CREATE ROLE testrole LOGIN NOSUPERUSER CREATEDB NOCREATEROLE
INHERIT NOREPLICATION NOBYPASSRLS CONNECTION LIMIT 5;
   ALTER ROLE testrole SET work_mem TO '64MB';

The patch includes regression tests covering various role configurations.

Co-authored-by: Mario Gonzalez and Bryan Green.

Comments?

BG

The rebased patch is attached.

Thanks,

--
Bryan Green
EDB: https://www.enterprisedb.com

On Fri, 7 Nov 2025 at 02:43, li carol <carol.li2025@outlook.com> wrote:

Hello Bryan,

I reviewed your patch and found one potential issue, please check it.
In pg_get_role_ddl_internal, the variable rolname is assigned from NameStr(roleform->rolname) (line 588), which means it points directly into the tuple returned from pg_authid. After constructing the initial CREATE ROLE statement, the code calls ReleaseSysCache(tuple); (line 665), so the memory holding that NameData now belongs to the cache again. However, the function continues to use rolname when building the subsequent ALTER ROLE statements (lines 756–765). Because the tuple has already been released, rolname is a dangling pointer and we risk reading garbage or crashing later. To fix this, copy the role name before releasing the syscache, e.g. rolname = pstrdup(NameStr(roleform->rolname));, and free it at the end.

Good catch, I didn't know NameStr returned a pointer, for some reason
I've assumed I was working with a copy. Attaching the patch with the
changes: (also I added you in "Reviewed-by")
diff --git a/src/backend/utils/adt/ruleutils.c
b/src/backend/utils/adt/ruleutils.c
index 584438d05ad..41db9f10f5d 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -585,7 +585,7 @@ pg_get_role_ddl_internal(Oid roleid)
                return NIL;

        roleform = (Form_pg_authid) GETSTRUCT(tuple);
-       rolname = NameStr(roleform->rolname);
+       rolname = pstrdup(NameStr(roleform->rolname));

/*
* We don't support generating DDL for system roles. The primary reason
@@ -777,6 +777,7 @@ pg_get_role_ddl_internal(Oid roleid)
table_close(rel, AccessShareLock);

pfree(buf.data);
+ pfree(rolname);

return statements;

https://cirrus-ci.com/build/4813271540170752

BR,
Yuan Li (Carol)

[...]

Co-authored-by: Mario Gonzalez and Bryan Green.

Comments?

BG

The rebased patch is attached.

Thanks,

--
Bryan Green
EDB: https://www.enterprisedb.com

--
Mario Gonzalez
EDB: https://www.enterprisedb.com

On Mon, 5 Jan 2026 at 07:17, Mario González Troncoso
<gonzalemario@gmail.com> wrote:

I reviewed your patch and found one potential issue, please check it.
In pg_get_role_ddl_internal, the variable rolname is assigned from NameStr(roleform->rolname) (line 588), which means it points directly into the tuple returned from pg_authid. After constructing the initial CREATE ROLE statement, the code calls ReleaseSysCache(tuple); (line 665), so the memory holding that NameData now belongs to the cache again. However, the function continues to use rolname when building the subsequent ALTER ROLE statements (lines 756–765). Because the tuple has already been released, rolname is a dangling pointer and we risk reading garbage or crashing later. To fix this, copy the role name before releasing the syscache, e.g. rolname = pstrdup(NameStr(roleform->rolname));, and free it at the end.

Good catch, I didn't know NameStr returned a pointer, for some reason
I've assumed I was working with a copy. Attaching the patch with the
changes: (also I added you in "Reviewed-by")

Hi, I'm reattaching after rebasing from master and fixing a conflict on:

 +++ b/src/test/regress/parallel_schedule
 @@ -125,6 +125,10 @@ test: plancache limit plpgsql copy2 temp domain
rangefuncs prepare conversion tr
  # ----------
- test: partition_join partition_prune reloptions hash_part indexing
partition_aggregate partition_info tuplesort explain compression
compression_lz4 memoize stats predicate numa eager_aggregate
+ test: partition_merge partition_split partition_join partition_prune
reloptions hash_part indexing partition_aggregate partition_info
tuplesort explain compression compression_lz4 memoize stats predicate
numa eager_aggregate

https://cirrus-ci.com/build/6142120160919552

Thanks.

diff --git a/src/backend/utils/adt/ruleutils.c
b/src/backend/utils/adt/ruleutils.c
index 584438d05ad..41db9f10f5d 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -585,7 +585,7 @@ pg_get_role_ddl_internal(Oid roleid)
return NIL;

roleform = (Form_pg_authid) GETSTRUCT(tuple);
-       rolname = NameStr(roleform->rolname);
+       rolname = pstrdup(NameStr(roleform->rolname));

/*
* We don't support generating DDL for system roles. The primary reason
@@ -777,6 +777,7 @@ pg_get_role_ddl_internal(Oid roleid)
table_close(rel, AccessShareLock);

--
Mario Gonzalez
EDB: https://www.enterprisedb.com

Hi, Mario González Troncoso

On Mon, 05 Jan 2026 at 07:29, Mario González Troncoso <gonzalemario@gmail.com> wrote:

On Mon, 5 Jan 2026 at 07:17, Mario González Troncoso
<gonzalemario@gmail.com> wrote:

I reviewed your patch and found one potential issue, please check it.
In pg_get_role_ddl_internal, the variable rolname is assigned from
NameStr(roleform->rolname) (line 588), which means it points
directly into the tuple returned from pg_authid. After
constructing the initial CREATE ROLE statement, the code calls
ReleaseSysCache(tuple); (line 665), so the memory holding that
NameData now belongs to the cache again. However, the function
continues to use rolname when building the subsequent ALTER ROLE
statements (lines 756–765). Because the tuple has already been
released, rolname is a dangling pointer and we risk reading
garbage or crashing later. To fix this, copy the role name before
releasing the syscache, e.g. rolname =
pstrdup(NameStr(roleform->rolname));, and free it at the end.

Good catch, I didn't know NameStr returned a pointer, for some reason
I've assumed I was working with a copy. Attaching the patch with the
changes: (also I added you in "Reviewed-by")

Hi, I'm reattaching after rebasing from master and fixing a conflict on:

+++ b/src/test/regress/parallel_schedule
@@ -125,6 +125,10 @@ test: plancache limit plpgsql copy2 temp domain
rangefuncs prepare conversion tr
# ----------
- test: partition_join partition_prune reloptions hash_part indexing
partition_aggregate partition_info tuplesort explain compression
compression_lz4 memoize stats predicate numa eager_aggregate
+ test: partition_merge partition_split partition_join partition_prune
reloptions hash_part indexing partition_aggregate partition_info
tuplesort explain compression compression_lz4 memoize stats predicate
numa eager_aggregate

https://cirrus-ci.com/build/6142120160919552

Thanks.

diff --git a/src/backend/utils/adt/ruleutils.c
b/src/backend/utils/adt/ruleutils.c
index 584438d05ad..41db9f10f5d 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -585,7 +585,7 @@ pg_get_role_ddl_internal(Oid roleid)
return NIL;

roleform = (Form_pg_authid) GETSTRUCT(tuple);
-       rolname = NameStr(roleform->rolname);
+       rolname = pstrdup(NameStr(roleform->rolname));

/*
* We don't support generating DDL for system roles. The primary reason
@@ -777,6 +777,7 @@ pg_get_role_ddl_internal(Oid roleid)
table_close(rel, AccessShareLock);

Thanks for updating the patch. Some comments on v4

1.

+       const char *separator = " ";
+
...
+       appendStringInfo(&buf, "%s%s", separator,
+                                        roleform->rolcanlogin ? "LOGIN" : "NOLOGIN");
+

The separator is never changed in pg_get_role_ddl_internal(), so we can remove
the variable and hard-code it in appendStringInfo().

2.
+       appendStringInfo(&buf, "%s%s", separator,
+                                        roleform->rolcanlogin ? "LOGIN" : "NOLOGIN");
+
+       appendStringInfo(&buf, "%s%s", separator,
+                                        roleform->rolsuper ? "SUPERUSER" : "NOSUPERUSER");
+
+       appendStringInfo(&buf, "%s%s", separator,
+                                        roleform->rolcreatedb ? "CREATEDB" : "NOCREATEDB");
+
+       appendStringInfo(&buf, "%s%s", separator,
+                                        roleform->rolcreaterole ? "CREATEROLE" : "NOCREATEROLE");
+
+       appendStringInfo(&buf, "%s%s", separator,
+                                        roleform->rolinherit ? "INHERIT" : "NOINHERIT");
+
+       appendStringInfo(&buf, "%s%s", separator,
+                                        roleform->rolreplication ? "REPLICATION" : "NOREPLICATION");
+
+       appendStringInfo(&buf, "%s%s", separator,
+                                        roleform->rolbypassrls ? "BYPASSRLS" : "NOBYPASSRLS");

For these options, I suggest preserving the same order as in the documentation.

postgres=# \h create role
Command: CREATE ROLE
Description: define a new database role
Syntax:
CREATE ROLE name [ [ WITH ] option [ ... ] ]

where option can be:

SUPERUSER | NOSUPERUSER
| CREATEDB | NOCREATEDB
| CREATEROLE | NOCREATEROLE
| INHERIT | NOINHERIT
| LOGIN | NOLOGIN
| REPLICATION | NOREPLICATION
| BYPASSRLS | NOBYPASSRLS
| CONNECTION LIMIT connlimit
| [ ENCRYPTED ] PASSWORD 'password' | PASSWORD NULL
| VALID UNTIL 'timestamp'
| IN ROLE role_name [, ...]
| ROLE role_name [, ...]
| ADMIN role_name [, ...]
| SYSID uid

3.

+       foreach(lc, statements)
+       {
+               char       *stmt = (char *) lfirst(lc);
+
+               if (!first)
+                       appendStringInfoChar(&result, '\n');
+               appendStringInfoString(&result, stmt);
+               first = false;
+       }

The foreach() macro can be replaced by foreach_ptr(), allowing us to remove
the lc variable entirely.

foreach_ptr(char, stmt, statements)
{
if (!first)
appendStringInfoChar(&result, '\n');
appendStringInfoString(&result, stmt);
first = false;
}

--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.

On Tue, 6 Jan 2026 at 03:27, Japin Li <japinli@hotmail.com> wrote:

[...]

Thanks for updating the patch. Some comments on v4

1.

+       const char *separator = " ";
+
...
+       appendStringInfo(&buf, "%s%s", separator,
+                                        roleform->rolcanlogin ? "LOGIN" : "NOLOGIN");
+

The separator is never changed in pg_get_role_ddl_internal(), so we can remove
the variable and hard-code it in appendStringInfo().

Is that what you mean by "remove the variable and hard-code"?

@@ -578,7 +578,6 @@ pg_get_role_ddl_internal(Oid roleid)
- const char *separator = " ";

tuple = SearchSysCache1(AUTHOID, ObjectIdGetDatum(roleid));
if (!HeapTupleIsValid(tuple))
@@ -605,34 +604,34 @@ pg_get_role_ddl_internal(Oid roleid)
* you'd typically write them in a CREATE ROLE command, though any order
* is actually acceptable to the parser.
*/

- appendStringInfo(&buf, "%s%s", separator,
-                                roleform->rolcanlogin ? "LOGIN" : "NOLOGIN");
-
- appendStringInfo(&buf, "%s%s", separator,
+ appendStringInfo(&buf, " %s",

The lines above are a snippet of the latest commit `WIP: removing
"separator"` on https://cirrus-ci.com/build/4621719253549056
Would you be able to see the whole change over there? If that's what
you mean, I'll squash afterwards and attach a new patch version to
this thread.

I applied the other feedback about foreach_ptr and to preserve the
order as in the docs, thanks!

2.
+       appendStringInfo(&buf, "%s%s", separator,
+                                        roleform->rolcanlogin ? "LOGIN" : "NOLOGIN");
+
+       appendStringInfo(&buf, "%s%s", separator,
+                                        roleform->rolsuper ? "SUPERUSER" : "NOSUPERUSER");
+
+       appendStringInfo(&buf, "%s%s", separator,
+                                        roleform->rolcreatedb ? "CREATEDB" : "NOCREATEDB");
+
+       appendStringInfo(&buf, "%s%s", separator,
+                                        roleform->rolcreaterole ? "CREATEROLE" : "NOCREATEROLE");
+
+       appendStringInfo(&buf, "%s%s", separator,
+                                        roleform->rolinherit ? "INHERIT" : "NOINHERIT");
+
+       appendStringInfo(&buf, "%s%s", separator,
+                                        roleform->rolreplication ? "REPLICATION" : "NOREPLICATION");
+
+       appendStringInfo(&buf, "%s%s", separator,
+                                        roleform->rolbypassrls ? "BYPASSRLS" : "NOBYPASSRLS");

For these options, I suggest preserving the same order as in the documentation.

postgres=# \h create role
Command: CREATE ROLE
Description: define a new database role
Syntax:
CREATE ROLE name [ [ WITH ] option [ ... ] ]

where option can be:

SUPERUSER | NOSUPERUSER
| CREATEDB | NOCREATEDB
| CREATEROLE | NOCREATEROLE
| INHERIT | NOINHERIT
| LOGIN | NOLOGIN
| REPLICATION | NOREPLICATION
| BYPASSRLS | NOBYPASSRLS
| CONNECTION LIMIT connlimit
| [ ENCRYPTED ] PASSWORD 'password' | PASSWORD NULL
| VALID UNTIL 'timestamp'
| IN ROLE role_name [, ...]
| ROLE role_name [, ...]
| ADMIN role_name [, ...]
| SYSID uid

3.

+       foreach(lc, statements)
+       {
+               char       *stmt = (char *) lfirst(lc);
+
+               if (!first)
+                       appendStringInfoChar(&result, '\n');
+               appendStringInfoString(&result, stmt);
+               first = false;
+       }

The foreach() macro can be replaced by foreach_ptr(), allowing us to remove
the lc variable entirely.

foreach_ptr(char, stmt, statements)
{
if (!first)
appendStringInfoChar(&result, '\n');
appendStringInfoString(&result, stmt);
first = false;
}

--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.

--
Mario Gonzalez
EDB: https://www.enterprisedb.com

On Wed, 07 Jan 2026 at 16:28, Mario González Troncoso <gonzalemario@gmail.com> wrote:

On Tue, 6 Jan 2026 at 03:27, Japin Li <japinli@hotmail.com> wrote:

[...]

Thanks for updating the patch. Some comments on v4

1.

+       const char *separator = " ";
+
...
+       appendStringInfo(&buf, "%s%s", separator,
+                                        roleform->rolcanlogin ? "LOGIN" : "NOLOGIN");
+

The separator is never changed in pg_get_role_ddl_internal(), so we can remove
the variable and hard-code it in appendStringInfo().

Is that what you mean by "remove the variable and hard-code"?

@@ -578,7 +578,6 @@ pg_get_role_ddl_internal(Oid roleid)
- const char *separator = " ";

tuple = SearchSysCache1(AUTHOID, ObjectIdGetDatum(roleid));
if (!HeapTupleIsValid(tuple))
@@ -605,34 +604,34 @@ pg_get_role_ddl_internal(Oid roleid)
* you'd typically write them in a CREATE ROLE command, though any order
* is actually acceptable to the parser.
*/

- appendStringInfo(&buf, "%s%s", separator,
-                                roleform->rolcanlogin ? "LOGIN" : "NOLOGIN");
-
- appendStringInfo(&buf, "%s%s", separator,
+ appendStringInfo(&buf, " %s",

The lines above are a snippet of the latest commit `WIP: removing
"separator"` on https://cirrus-ci.com/build/4621719253549056
Would you be able to see the whole change over there? If that's what
you mean, I'll squash afterwards and attach a new patch version to
this thread.

Yeah, you read my mind.

--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.

On Wed, 7 Jan 2026 at 22:40, Japin Li <japinli@hotmail.com> wrote:

Is that what you mean by "remove the variable and hard-code"?

@@ -578,7 +578,6 @@ pg_get_role_ddl_internal(Oid roleid)
- const char *separator = " ";

tuple = SearchSysCache1(AUTHOID, ObjectIdGetDatum(roleid));
if (!HeapTupleIsValid(tuple))
@@ -605,34 +604,34 @@ pg_get_role_ddl_internal(Oid roleid)
* you'd typically write them in a CREATE ROLE command, though any order
* is actually acceptable to the parser.
*/

- appendStringInfo(&buf, "%s%s", separator,
-                                roleform->rolcanlogin ? "LOGIN" : "NOLOGIN");
-
- appendStringInfo(&buf, "%s%s", separator,
+ appendStringInfo(&buf, " %s",

The lines above are a snippet of the latest commit `WIP: removing
"separator"` on https://cirrus-ci.com/build/4621719253549056
Would you be able to see the whole change over there? If that's what
you mean, I'll squash afterwards and attach a new patch version to
this thread.

Yeah, you read my mind.

Cool. I rebased this morning and it passed just fine.

--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.

--
Mario Gonzalez
EDB: https://www.enterprisedb.com

Hi

Date: Fri, 09 Jan 2026 13:58:09 +0800

On Thu, 08 Jan 2026 at 09:19, Mario González Troncoso <gonzalemario@gmail.com> wrote:

On Wed, 7 Jan 2026 at 22:40, Japin Li <japinli@hotmail.com> wrote:

Is that what you mean by "remove the variable and hard-code"?

@@ -578,7 +578,6 @@ pg_get_role_ddl_internal(Oid roleid)
- const char *separator = " ";

tuple = SearchSysCache1(AUTHOID, ObjectIdGetDatum(roleid));
if (!HeapTupleIsValid(tuple))
@@ -605,34 +604,34 @@ pg_get_role_ddl_internal(Oid roleid)
* you'd typically write them in a CREATE ROLE command, though any order
* is actually acceptable to the parser.
*/

- appendStringInfo(&buf, "%s%s", separator,
-                                roleform->rolcanlogin ? "LOGIN" : "NOLOGIN");
-
- appendStringInfo(&buf, "%s%s", separator,
+ appendStringInfo(&buf, " %s",

The lines above are a snippet of the latest commit `WIP: removing
"separator"` on https://cirrus-ci.com/build/4621719253549056
Would you be able to see the whole change over there? If that's what
you mean, I'll squash afterwards and attach a new patch version to
this thread.

Yeah, you read my mind.

Cool. I rebased this morning and it passed just fine.

Thanks for updating the patch. Here are some more comments.

1.
Why not handle the [IN] ROLE and ADMIN clauses? Is there something I missed?

2.
+ * Returns NIL if the role OID is invalid.  This can happen if the role was
+ * dropped concurrently, or if we're passed a OID that doesn't match
+ * any role.

However, when I tested concurrent DROP ROLE, the function can still return a
non-NIL result (though incomplete).

Here’s a reproducible scenario:

a) Prepare
-- Session 1
CREATE USER u01 WITH CONNECTION LIMIT 10;
ALTER USER u01 IN DATABASE postgres SET work_mem TO '16MB';
SELECT pg_get_role_ddl_statements('u01'::regrole);

b) Set a breakpoint in Session 1's backend using GDB at pg_get_role_ddl_internal.

c) Execute the query in Session 1:
   --- Session 1
   SELECT pg_get_role_ddl_statements('u01'::regrole);

d) In GDB, step over the line:
tuple = SearchSysCache1(AUTHOID, ObjectIdGetDatum(roleid));

e) In Session 2, drop the user:
   --- Session 2
   DROP USER u01;

f) Continue execution in GDB.

Result in Session 1:

postgres=# SELECT pg_get_role_ddl_statements('u01'::regrole);
pg_get_role_ddl_statements
------------------------------------------------------------------------------------------------------------------
CREATE ROLE u01 NOSUPERUSER NOCREATEDB NOCREATEROLE INHERIT LOGIN NOREPLICATION NOBYPASSRLS CONNECTION LIMIT 10;
(1 row)

We only get the CREATE ROLE statement; the ALTER ROLE ... SET work_mem
statement is missing. This behavior does not fully match the comment, which
implies that an invalid OID would return NIL. In this case, we get a partial
(and potentially misleading) result instead.

--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.

On Thu, Jan 8, 2026 at 8:20 PM Mario González Troncoso
<gonzalemario@gmail.com> wrote:

Cool. I rebased this morning and it passed just fine.

hi.

+ /*
+ * We don't support generating DDL for system roles.  The primary reason
+ * for this is that users shouldn't be recreating them.
+ */
+ if (strncmp(rolname, "pg_", 3) == 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_RESERVED_NAME),
+ errmsg("role name \"%s\" is reserved", rolname),
+ errdetail("Role names starting with \"pg_\" are reserved for system
roles.")));

There is a specific function for it, IsReservedName,
we should use it instead of strncmp, i think.

+ /*
+ * We intentionally omit PASSWORD.  There's no way to retrieve the
+ * original password text from the stored hash, and even if we could,
+ * exposing passwords through a SQL function would be a security issue.
+ * Users must set passwords separately after recreating roles.
+ */
+
+ appendStringInfoChar(&buf, ';');

What about the PASSWORD NULL scarenio?

per Synopsis:
CREATE ROLE name [ [ WITH ] option [ ... ] ]

Do we need to include the optional keyword, WITH?

--
jian
https://www.enterprisedb.com/

On Fri, Jan 9, 2026 at 2:05 PM Japin Li <japinli@hotmail.com> wrote:

2.
+ * Returns NIL if the role OID is invalid.  This can happen if the role was
+ * dropped concurrently, or if we're passed a OID that doesn't match
+ * any role.

However, when I tested concurrent DROP ROLE, the function can still return a
non-NIL result (though incomplete).

Here’s a reproducible scenario:

a) Prepare
-- Session 1
CREATE USER u01 WITH CONNECTION LIMIT 10;
ALTER USER u01 IN DATABASE postgres SET work_mem TO '16MB';
SELECT pg_get_role_ddl_statements('u01'::regrole);

b) Set a breakpoint in Session 1's backend using GDB at pg_get_role_ddl_internal.

c) Execute the query in Session 1:
--- Session 1
SELECT pg_get_role_ddl_statements('u01'::regrole);

d) In GDB, step over the line:
tuple = SearchSysCache1(AUTHOID, ObjectIdGetDatum(roleid));

e) In Session 2, drop the user:
--- Session 2
DROP USER u01;

f) Continue execution in GDB.

Result in Session 1:

postgres=# SELECT pg_get_role_ddl_statements('u01'::regrole);
pg_get_role_ddl_statements
------------------------------------------------------------------------------------------------------------------
CREATE ROLE u01 NOSUPERUSER NOCREATEDB NOCREATEROLE INHERIT LOGIN NOREPLICATION NOBYPASSRLS CONNECTION LIMIT 10;
(1 row)

We only get the CREATE ROLE statement; the ALTER ROLE ... SET work_mem
statement is missing. This behavior does not fully match the comment, which
implies that an invalid OID would return NIL. In this case, we get a partial
(and potentially misleading) result instead.

+ tuple = SearchSysCache1(AUTHOID, ObjectIdGetDatum(roleid));
+ if (!HeapTupleIsValid(tuple))
+ return NIL;

after SearchSysCache1, HeapTupleIsValid, adding

+ LockSharedObject(AuthIdRelationId, roleid, 0, AccessShareLock);

can solve this problem.
We have a similar code pattern in DropRole.

--
jian
https://www.enterprisedb.com/

On Thu, Jan 8, 2026 at 8:20 PM Mario González Troncoso
<gonzalemario@gmail.com> wrote:

Cool. I rebased this morning and it passed just fine.

hi.

--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -12683,4 +12683,28 @@
   proname => 'hashoid8extended', prorettype => 'int8',
   proargtypes => 'oid8 int8', prosrc => 'hashoid8extended' },

+# pg_get_role_ddl - return DDL to recreate a role (compact format)
+{ oid => '9991', descr => 'get SQL commands to recreate role',
+  proname => 'pg_get_role_ddl', pronamespace => 'pg_catalog',
+  proowner => 'POSTGRES', prolang => 'internal', procost => '1',
prorows => '0',
+  provariadic => '0', prosupport => '0', prokind => 'f', prosecdef => 'f',
+  proleakproof => 'f', proisstrict => 't', proretset => 'f',
provolatile => 's',
+  proparallel => 's', pronargs => '1', pronargdefaults => '0',
+  prorettype => 'text', proargtypes => 'regrole', proallargtypes => '_null_',
+  proargmodes => '_null_', proargnames => '_null_', proargdefaults => '_null_',
+  protrftypes => '_null_', prosrc => 'pg_get_role_ddl', probin => '_null_',
+  prosqlbody => '_null_', proconfig => '_null_', proacl => '_null_' },
+
+# pg_get_role_ddl_statements - return DDL as separate statements
(compact format)
+{ oid => '9992', descr => 'get SQL commands to recreate role as row set',
+  proname => 'pg_get_role_ddl_statements', pronamespace => 'pg_catalog',
+  proowner => 'POSTGRES', prolang => 'internal', procost => '1',
+  prorows => '10', provariadic => '0', prosupport => '0', prokind => 'f',
+  prosecdef => 'f', proleakproof => 'f', proisstrict => 't', proretset => 't',
+  provolatile => 's', proparallel => 's', pronargs => '1',
+  pronargdefaults => '0', prorettype => 'text', proargtypes => 'regrole',
+  proallargtypes => '_null_', proargmodes => '_null_', proargnames => '_null_',
+  proargdefaults => '_null_', protrftypes => '_null_',
+  prosrc => 'pg_get_role_ddl_statements', probin => '_null_',
+  prosqlbody => '_null_', proconfig => '_null_', proacl => '_null_' },
 ]'

you don't need to specify proowner.
if you want these two functions only available for superuser, you can
change it at
src/backend/catalog/system_functions.sql, see line 660.

many other fields also no need explicitly specified, see
src/include/catalog/pg_proc.h,
BKI_DEFAULT.

create_role.sql, roleattributes.sql have so many CREATE ROLE statements. we can
just add more tests on these two files, no need for extra
src/test/regress/sql/role_ddl.sql file, IMHO.

Hi, jian

On Sat, 17 Jan 2026 at 11:13, jian he <jian.universality@gmail.com> wrote:

On Fri, Jan 9, 2026 at 2:05 PM Japin Li <japinli@hotmail.com> wrote:

2.
+ * Returns NIL if the role OID is invalid.  This can happen if the role was
+ * dropped concurrently, or if we're passed a OID that doesn't match
+ * any role.

However, when I tested concurrent DROP ROLE, the function can still return a
non-NIL result (though incomplete).

Here’s a reproducible scenario:

a) Prepare
-- Session 1
CREATE USER u01 WITH CONNECTION LIMIT 10;
ALTER USER u01 IN DATABASE postgres SET work_mem TO '16MB';
SELECT pg_get_role_ddl_statements('u01'::regrole);

b) Set a breakpoint in Session 1's backend using GDB at pg_get_role_ddl_internal.

c) Execute the query in Session 1:
--- Session 1
SELECT pg_get_role_ddl_statements('u01'::regrole);

d) In GDB, step over the line:
tuple = SearchSysCache1(AUTHOID, ObjectIdGetDatum(roleid));

e) In Session 2, drop the user:
--- Session 2
DROP USER u01;

f) Continue execution in GDB.

Result in Session 1:

postgres=# SELECT pg_get_role_ddl_statements('u01'::regrole);
pg_get_role_ddl_statements
------------------------------------------------------------------------------------------------------------------
CREATE ROLE u01 NOSUPERUSER NOCREATEDB NOCREATEROLE INHERIT LOGIN NOREPLICATION NOBYPASSRLS CONNECTION LIMIT 10;
(1 row)

We only get the CREATE ROLE statement; the ALTER ROLE ... SET work_mem
statement is missing. This behavior does not fully match the comment, which
implies that an invalid OID would return NIL. In this case, we get a partial
(and potentially misleading) result instead.

+ tuple = SearchSysCache1(AUTHOID, ObjectIdGetDatum(roleid));
+ if (!HeapTupleIsValid(tuple))
+ return NIL;

after SearchSysCache1, HeapTupleIsValid, adding

+ LockSharedObject(AuthIdRelationId, roleid, 0, AccessShareLock);

Thanks for pointing this out.

can solve this problem.
We have a similar code pattern in DropRole.

I think this can handle most cases, but there is still a small window where a
concurrent DROP USER could happen between SearchSysCache1() and LockSharedObject().

--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.