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

From Mario González Troncoso
Subject Re: [PATCH] Add pg_get_role_ddl() functions for role recreation
Date
Msg-id CAFsReFXJ6dxawcDcdcHpwnzhFO7T5j2BZhZ1eq1MC4e2dPpxPw@mail.gmail.com
Whole thread Raw
In response to Re: [PATCH] Add pg_get_role_ddl() functions for role recreation  (Mario González Troncoso <gonzalemario@gmail.com>)
List pgsql-hackers
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
meansit points directly into the tuple returned from pg_authid.  After constructing the initial CREATE ROLE statement,
thecode 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).
Becausethe 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

Attachment

pgsql-hackers by date:

Previous
From: Prafulla Ranadive
Date:
Subject: Re: Need help with postgresql build on windows
Next
From: shveta malik
Date:
Subject: Re: [Patch] add new parameter to pg_replication_origin_session_setup