Thread

1. Cast to regrole on a literal string in a PL/pgSQL function

   LEMAIRE Leslie - SG/DNUM/UNI/DRC (Chargée de mission) <leslie.lemaire@developpement-durable.gouv.fr> — 2025-09-29T12:36:15Z

   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
   
   
   
   

2. Re: Cast to regrole on a literal string in a PL/pgSQL function

   Tom Lane <tgl@sss.pgh.pa.us> — 2025-09-29T17:34:33Z

   =?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
   
   
   
   

3. Re: Cast to regrole on a literal string in a PL/pgSQL function

   Tom Lane <tgl@sss.pgh.pa.us> — 2025-09-29T22:25:44Z

   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
   
   
   
   

4. Re: Cast to regrole on a literal string in a PL/pgSQL function

   LEMAIRE Leslie - SG/DNUM/UNI/DRC (Chargée de mission) <leslie.lemaire@developpement-durable.gouv.fr> — 2025-09-30T09:25:46Z

   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
   
   
   
   

5. Re: Cast to regrole on a literal string in a PL/pgSQL function

   Tom Lane <tgl@sss.pgh.pa.us> — 2025-09-30T15:29:51Z

   =?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