Re: BUG #18059: Unexpected error 25001 in stored procedure

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Sat, Aug 19, 2023 at 1:19 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> What I'm inclined to propose, therefore, is that we make revalidation
>> be a no-op for every statement type for which transformStmt() reaches
>> its default: case. (When it does so, the resulting CMD_UTILITY Query
>> will not get any processing from the rewriter or planner either.)

> That sounds like the right thing. It is perhaps unfortunate that we
> don't have a proper parse analysis/execution distinction for other
> types of statements, but if that ever changes then this can be
> revisited.

I started to code this, and immediately noticed that transformStmt()
already has a companion function analyze_requires_snapshot() that
returns "true" in the cases of interest ... except that it does
not return true for T_CallStmt. Perhaps that was intentional to
begin with, but it is very hard to believe that it isn't a bug now,
since transformCallStmt can invoke nearly arbitrary processing via
transformExpr(). What semantic anomalies, if any, do we risk if CALL
processing forces a transaction start? (I rather imagine it does
already, somewhere later on...)

Anyway, I'm now of two minds whether to use analyze_requires_snapshot()
as-is for plancache.c's invalidation test, or duplicate it under a
different name, or have two names but one is just an alias for the
other. It still seems like "analyze requires snapshot" isn't
necessarily the exact inverse condition of "analyze is a no-op", but
it is today (assuming we agree that CALL needs a snapshot), and maybe
maintaining two duplicate functions is silly. Thoughts?

regards, tom lane