Re: SQLFunctionCache and generic plans

I wrote:
> There's more stuff that could be done, but I feel that all of this
> could be left for later:
> * I really wanted to do what I mentioned upthread and change things
> so we don't even parse the later queries until we've executed the
> ones before that. However that seems to be a bit of a mess to
> make happen, and the patch is large/complex enough already.

I felt more regret about that decision after reading the discussion
at [1] and realizing that we already moved those goalposts part way.
For example, historically this fails:

set check_function_bodies to off;

create or replace function create_and_insert() returns void
language sql as $$
create table t1 (f1 int);
insert into t1 values (1.2);
$$;

select create_and_insert();

You get "ERROR: relation "t1" does not exist" because we try to
parse-analyze the INSERT before the CREATE has been executed.
That's still true with patch v10. However, consider this:

create table t1 (f1 int);

create or replace function alter_and_insert() returns void
language sql as $$
alter table t1 alter column f1 type numeric;
insert into t1 values (1.2);
$$;

select alter_and_insert();

Historically that fails with

ERROR: table row type and query-specified row type do not match
DETAIL: Table has type numeric at ordinal position 1, but query expects integer.
CONTEXT: SQL function "alter_and_insert" statement 2

because we built a plan for the INSERT before executing the ALTER.
However, with v10 it works! The INSERT is parse-analyzed against the
old table definition, but that's close enough that we can successfully
make a CachedPlanSource. Then when we come to execute the INSERT,
the plancache notices that what it has is already invalidated,
so it re-does everything and the correct data type is used.

So that left me quite unsatisfied. It's not great to modify edge-case
semantics a little bit and then come back and modify them some more in
the next release; better for it to all happen at once. Besides which,
it'd be quite difficult to document v10's behavior in a fashion that
makes any sense to users. I'd abandoned the idea of delaying parse
analysis over the weekend because time was running out and I had
enough things to worry about, but I resolved to take another look.
And it turns out not to be very hard at all to get there from where
we were in v10: we already did 90% of the restructuring needed to
do the processing incrementally. The main missing thing is we need
someplace to stash the raw parse trees or Queries we got from pg_proc
until we are ready to do the analyze-rewrite-and-make-a-cached-plan
business for them. I had already had the idea of making a second
context inside the SQLFunctionHashEntry to keep those trees in
until we don't need them anymore (which we don't once the last
CachedPlanSource is made). So that part wasn't hard. I then
discovered that the error reporting needed some tweaking, which
wasn't hard either.

So attached is v11. 0001-0006 are identical to v10, and then
0007 is the delta. (I'd plan to squash that in final commit,
but I thought it'd be easier to review this way.)

Compared to v10, this means that parse analysis work as well as
planning work is done in fcontext, so there's a little bit more
temporary leakage during the first run of a lazy-evaluation SRF.
I still think that this is not going to amount to anything worth
worrying about, but maybe it slightly raises the interest level
of not doing everything in fcontext.

Another point is that the order of the subroutines in functions.c
is starting to feel rather random. I left them like this to
minimize the amount of pure code motion involved in 0007, but
I'm tempted to rearrange them into something closer to
order-of-execution before final commit.

Anyway, I feel pretty good about this patch now and am quite content
to stop here for PG 18.

regards, tom lane

[1] https://www.postgresql.org/message-id/flat/4115257.1743441667%40sss.pgh.pa.us#ae47bbce3b234245c530ad06eb819d8c