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

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