Re: Cast to regrole on a literal string in a PL/pgSQL function - Mailing list pgsql-bugs

From LEMAIRE Leslie (Chargée de mission) - SG/DNUM/UNI/DRC
Subject Re: Cast to regrole on a literal string in a PL/pgSQL function
Date
Msg-id 2d723791540f2bdbd76da34d7e90f8ee@developpement-durable.gouv.fr
Whole thread Raw
In response to Re: Cast to regrole on a literal string in a PL/pgSQL function  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Cast to regrole on a literal string in a PL/pgSQL function
List pgsql-bugs
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



pgsql-bugs by date:

Previous
From: Erki Eessaar
Date:
Subject: Potential bug: Enforcing/not enforcing a CHECK constraint fails on an empty table
Next
From: David Rowley
Date:
Subject: Re: Potential bug: Enforcing/not enforcing a CHECK constraint fails on an empty table