Обсуждение: Cast to regrole on a literal string in a PL/pgSQL function

Hi,

I've noticed a rather odd behavior with a PL/pgSQL function whose 
definition includes a cast to regrole on a literal string, such as 
'a_role'::regrole.

The following function simply creates a role "a_role", asserts that the 
result of a cast to regrole on the string 'a_role' returns the same OID 
as the actual OID registered in pg_roles for "a_role", and finally drops 
the role. When this function is executed repeatedly, through separate 
transactions or not, but with the same connection, the assertion will 
unexpectedly fail after the first try, because 'a_role'::regrole keeps 
returning the OID that was given to the now-dropped role on the first 
try, instead of the actual OID of the newly created role.

I first observed this issue with a PostgreSQL 17.6 server on Debian - 
Debian 17.6-1.pgdg12+1 on x86_64-pc-linux-gnu, compiled by gcc (Debian 
12.2.0-14+deb12u1) 12.2.0, 64-bit -, then reproduced it with PostgreSQL 
13 to 18 on Windows.

CREATE OR REPLACE FUNCTION regrole_cast_anomaly()
     RETURNS VOID
     LANGUAGE plpgsql
     AS $_$
     BEGIN
         -- with a table
         -- this test won't fail
         CREATE TABLE a_table () ;

         ASSERT 'a_table'::regclass = (
             SELECT pg_class.oid::regclass
                 FROM pg_catalog.pg_class
                 WHERE pg_class.relname = 'a_table'
         ), 'something''s wrong with ''a_table''::regclass' ;

         DROP TABLE a_table ;

         -- with a role
         -- this test is very likely to fail if the function
         -- is run more than once
         CREATE ROLE a_role ;

         RAISE NOTICE '[1] ''a_role''::regrole = %', 'a_role'::regrole ;
         RAISE NOTICE '[2] %', (
             SELECT format('actual OID = %s', pg_roles.oid)
                 FROM pg_catalog.pg_roles
                 WHERE pg_roles.rolname = 'a_role'
         ) ;

         ASSERT 'a_role'::regrole = (
             SELECT pg_roles.oid::regrole
                 FROM pg_catalog.pg_roles
                 WHERE pg_roles.rolname = 'a_role'
         ), 'something''s wrong with ''a_role''::regrole' ;

         DROP ROLE a_role ;
     END
     $_$ ;
SELECT regrole_cast_anomaly() ;
SELECT regrole_cast_anomaly() ; -- the assertion will fail

I couldn't find any warning about this in the documentation.

Thank you in advance for looking into it.

Regards,
Leslie Lemaire
Secrétariat général des ministères en charge de l'aménagement du 
territoire et de la transition écologique
Direction du numérique

=?UTF-8?Q?LEMAIRE_Leslie_=28Charg=C3=A9e_de_mission=29_=2D_SG=2FDNUM=2F?= =?UTF-8?Q?UNI=2FDRC?=
<leslie.lemaire@developpement-durable.gouv.fr>writes: 
> I've noticed a rather odd behavior with a PL/pgSQL function whose
> definition includes a cast to regrole on a literal string, such as
> 'a_role'::regrole.

Yeah, the coverage for REG* constants in plan invalidation is pretty
thin --- in fact, I think this *only* works correctly for regclass
constants.  AFAIR you're the first to complain, so I'm not sure that
we want to expend the effort to expand that ...

            regards, tom lane

I wrote:
> Yeah, the coverage for REG* constants in plan invalidation is pretty
> thin --- in fact, I think this *only* works correctly for regclass
> constants.  AFAIR you're the first to complain, so I'm not sure that
> we want to expend the effort to expand that ...

I spent a little bit of time poking into what that would involve.
The two key bits are that setrefs.c's fix_expr_common() only collects
dependency data for regclass constants not other reg* cases, and
that plancache.c doesn't have dependency-matching infrastructure
except for relations, functions, and types.  So we'd have to expand
both of those parts to make other kinds of reg* constants work nicely.

The main stumbling block to expanding the data-collection aspect
is this kluge in setrefs.c:

/*
 * Check if a Const node is a regclass value.  We accept plain OID too,
 * since a regclass Const will get folded to that type if it's an argument
 * to oideq or similar operators.  (This might result in some extraneous
 * values in a plan's list of relation dependencies, but the worst result
 * would be occasional useless replans.)
 */
#define ISREGCLASSCONST(con) \
    (((con)->consttype == REGCLASSOID || (con)->consttype == OIDOID) && \
     !(con)->constisnull)

I can't see recording an OID-type constant as being a potential match
to every reg* category: that would slow down plancache.c's invalidation
callbacks and probably result in a lot of useless invalidations.
So we'd have to up our game about how this is done.  On reflection
it seems to me that it would be okay to capture these dependencies
earlier, in eval_const_expressions: if we see a Const of any reg*
type, capture that dependency before we start folding anything.
We do not have to worry about any case other than a reg* Const
produced by the parser, because for example cases like
    ('role' || '_a')::regrole
will not be folded to Consts since regrolein is not immutable.
This isn't an enormous expansion of eval_const_expressions' charter
either: it already has to capture dependencies in some cases, such
as when it's inlined a user-defined function.

The other thing that bothers me a bit is that plancache.c would have
to establish invalidation callbacks on quite a few more syscaches
than it has currently.  Will that be a performance problem, and if
so what could we do about it?


A totally different way of thinking about this is that it's a mistake
for the parser to ever produce reg* Consts in the first place, or
indeed Consts for any datatype with a non-immutable input function.
This has been discussed before, and there are comments about it in
coerce_type() where the deed is done.  If we made a Const of type
cstring and then applied the input function as a FuncExpr, then
we'd not try to evaluate it till runtime.  This answer has a great
deal of intellectual purity to it, but I'm kind of afraid of the
consequences of making such a change: both edge-case changes in
semantic behavior and potentially severe performance degradation
could ensue.  I doubt that reg* values themselves would be a big
deal, but other types with non-immutable input functions such as
timestamptz are definitely performance-critical in many applications.
So, purity or not, the potential blast radius of this answer seems
uncomfortably large.


Anyway, I'm not excited enough by this problem to expend more time
on it now; I'm just recording this brain dump in case somebody
else would like to pursue it.  The workaround I'd recommend in the
meantime is to use to_regrole() instead of a cast, or else write
the cast like 'role_a'::text::regrole.  Either of those will ensure
that the constant embedded in the expression's cached plan will
just be a string not an OID.

            regards, tom lane

Thank you for your very detailed answer, and for considering the 
possibility of fixing this issue. I understand why you don't want to do 
it for now.

Actually, as a user, I never really need this specific form of cast in 
my functions. As you said, there are alternatives. I just happened to 
use it, and a lot, because I was allowed to, because it was shorter, and 
because I didn't know that it could cause my functions to (silently) 
malfunction.

That surely wouldn't be enough, but maybe a clearer warning in the 
documentation would help a little, for instance in the "Object 
Identifier Types" page. Raising an error when a PL/pgSQL function 
performs this kind of not-quite-supported reg* cast on a literal string 
could be an option as well (excluding regclass, since it works). That 
would be a breaking change for some, including me, but it would be 
cleaner. I don't think I'm the only one who would rather fix my 
functions than risk anomalies later on.

I'm surprised that regnamespace isn't handled properly either, by the 
way. One test can't prove much, but this one doesn't fail with a cast to 
regnamespace, unlike all other reg* except regclass.

Regards,

Leslie Lemaire

Le 30/09/2025 00:25, > tgl a écrit :
> I wrote:
>> Yeah, the coverage for REG* constants in plan invalidation is pretty
>> thin --- in fact, I think this *only* works correctly for regclass
>> constants.  AFAIR you're the first to complain, so I'm not sure that
>> we want to expend the effort to expand that ...
> 
> I spent a little bit of time poking into what that would involve.
> The two key bits are that setrefs.c's fix_expr_common() only collects
> dependency data for regclass constants not other reg* cases, and
> that plancache.c doesn't have dependency-matching infrastructure
> except for relations, functions, and types.  So we'd have to expand
> both of those parts to make other kinds of reg* constants work nicely.
> 
> The main stumbling block to expanding the data-collection aspect
> is this kluge in setrefs.c:
> 
> /*
>  * Check if a Const node is a regclass value.  We accept plain OID too,
>  * since a regclass Const will get folded to that type if it's an 
> argument
>  * to oideq or similar operators.  (This might result in some 
> extraneous
>  * values in a plan's list of relation dependencies, but the worst 
> result
>  * would be occasional useless replans.)
>  */
> #define ISREGCLASSCONST(con) \
>     (((con)->consttype == REGCLASSOID || (con)->consttype == OIDOID) && \
>      !(con)->constisnull)
> 
> I can't see recording an OID-type constant as being a potential match
> to every reg* category: that would slow down plancache.c's invalidation
> callbacks and probably result in a lot of useless invalidations.
> So we'd have to up our game about how this is done.  On reflection
> it seems to me that it would be okay to capture these dependencies
> earlier, in eval_const_expressions: if we see a Const of any reg*
> type, capture that dependency before we start folding anything.
> We do not have to worry about any case other than a reg* Const
> produced by the parser, because for example cases like
>     ('role' || '_a')::regrole
> will not be folded to Consts since regrolein is not immutable.
> This isn't an enormous expansion of eval_const_expressions' charter
> either: it already has to capture dependencies in some cases, such
> as when it's inlined a user-defined function.
> 
> The other thing that bothers me a bit is that plancache.c would have
> to establish invalidation callbacks on quite a few more syscaches
> than it has currently.  Will that be a performance problem, and if
> so what could we do about it?
> 
> 
> A totally different way of thinking about this is that it's a mistake
> for the parser to ever produce reg* Consts in the first place, or
> indeed Consts for any datatype with a non-immutable input function.
> This has been discussed before, and there are comments about it in
> coerce_type() where the deed is done.  If we made a Const of type
> cstring and then applied the input function as a FuncExpr, then
> we'd not try to evaluate it till runtime.  This answer has a great
> deal of intellectual purity to it, but I'm kind of afraid of the
> consequences of making such a change: both edge-case changes in
> semantic behavior and potentially severe performance degradation
> could ensue.  I doubt that reg* values themselves would be a big
> deal, but other types with non-immutable input functions such as
> timestamptz are definitely performance-critical in many applications.
> So, purity or not, the potential blast radius of this answer seems
> uncomfortably large.
> 
> 
> Anyway, I'm not excited enough by this problem to expend more time
> on it now; I'm just recording this brain dump in case somebody
> else would like to pursue it.  The workaround I'd recommend in the
> meantime is to use to_regrole() instead of a cast, or else write
> the cast like 'role_a'::text::regrole.  Either of those will ensure
> that the constant embedded in the expression's cached plan will
> just be a string not an OID.
> 
>             regards, tom lane

=?UTF-8?Q?LEMAIRE_Leslie_=28Charg=C3=A9e_de_mission=29_=2D_SG=2FDNUM=2F?= =?UTF-8?Q?UNI=2FDRC?=
<leslie.lemaire@developpement-durable.gouv.fr>writes: 
> I'm surprised that regnamespace isn't handled properly either, by the
> way. One test can't prove much, but this one doesn't fail with a cast to
> regnamespace, unlike all other reg* except regclass.

regnamespace constants do work, accidentally, because plancache.c
is set up to flush *all* cached plans after any change in
pg_namespace.  That's because the possible effects on search-path
lookups are too hard to predict.

Now that I look, regoper/regoperator constants would probably be okay
too, because changes in pg_operator also cause a plancache flush.

            regards, tom lane