Re: [PATCH] Add pg_get_role_ddl() functions for role recreation - Mailing list pgsql-hackers

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.