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: