Re: Support prepared statement invalidation when result types change

On Mon, 18 Sept 2023 at 18:01, Jelte Fennema-Nio <me(at)jeltef(dot)nl> wrote:
>
> @Euler thanks for the review. I addressed the feedback.
>
> On Fri, 15 Sept 2023 at 01:41, Andy Fan <zhihui(dot)fan1213(at)gmail(dot)com> wrote:
> > What if a client has *cached* an old version of RowDescription
> > and the server changed it to something new and sent resultdata
> > with the new RowDescription. Will the client still be able to work
> > expectly?
>
> It depends a bit on the exact change. For instance a column being
> added to the end of the resultdata shouldn't be a problem. And that is
> actually quite a common case for this issue:
> 1. PREPARE p as (SELECT * FROM t);
> 2. ALTER TABLE t ADD COLUMN ...
> 3. EXECUTE p
>
> But type changes of existing columns might cause issues when the
> RowDescription is cached. But such changes also cause issues now.
> Currently the prepared statement becomes unusable when this happens
> (returning errors every time). With this patch it's at least possible
> to have prepared statements continue working in many cases.
> Furthermore caching RowDescription is also not super useful, most
> clients request it every time because it does not require an extra
> round trip, so there's almost no overhead in requesting it.
>
> Clients caching ParameterDescription seems more useful because
> fetching the parameter types does require an extra round trip. So
> caching it could cause errors with 0003. But right now if the argument
> types need to change it gives an error every time when executing the
> prepared statement. So I believe 0003 is still an improvement over the
> status quo, because there are many cases where the client knows that
> the types might have changed and it thus needs to re-fetch the
> ParameterDescription: the most common case is changing the
> search_path. And there's also cases where even a cached
> ParamaterDescription will work fine: e.g. the type is changed but the
> encoding stays the same (e.g. drop + create an enum, or text/varchar,
> or the text encoding of int and bigint)

One of the test has aborted in CFBot at [1] with:
[05:26:16.214] Core was generated by `postgres: postgres
regression_pg_stat_statements [local] EXECUTE '.
[05:26:16.214] Program terminated with signal SIGABRT, Aborted.
[05:26:16.214] #0 __GI_raise (sig=sig(at)entry=6) at
../sysdeps/unix/sysv/linux/raise.c:50
[05:26:16.214] Download failed: Invalid argument. Continuing without
source file ./signal/../sysdeps/unix/sysv/linux/raise.c.
[05:26:16.392]
[05:26:16.392] Thread 1 (Thread 0x7fbe1d997a40 (LWP 28738)):
[05:26:16.392] #0 __GI_raise (sig=sig(at)entry=6) at
../sysdeps/unix/sysv/linux/raise.c:50
....
....
[05:26:36.911] #5 0x000055c5aa523e71 in RevalidateCachedQuery
(plansource=0x55c5ac811cf0, queryEnv=queryEnv(at)entry=0x0) at
../src/backend/utils/cache/plancache.c:730
[05:26:36.911] num_params = 0
[05:26:36.911] param_types = 0x55c5ac860438
[05:26:36.911] snapshot_set = false
[05:26:36.911] rawtree = 0x55c5ac859f08
[05:26:36.911] tlist = <optimized out>
[05:26:36.911] qlist = <optimized out>
[05:26:36.911] resultDesc = <optimized out>
[05:26:36.911] querytree_context = <optimized out>
[05:26:36.911] oldcxt = <optimized out>
[05:26:36.911] #6 0x000055c5a9de0262 in ExplainExecuteQuery
(execstmt=0x55c5ac6d9d38, into=0x0, es=0x55c5ac859648,
queryString=0x55c5ac6d91e0 "EXPLAIN (VERBOSE, COSTS OFF) EXECUTE
st6;", params=0x0, queryEnv=0x0) at
../src/backend/commands/prepare.c:594
[05:26:36.911] entry = 0x55c5ac839ba8
[05:26:36.911] query_string = <optimized out>
[05:26:36.911] cplan = <optimized out>
[05:26:36.911] plan_list = <optimized out>
[05:26:36.911] p = <optimized out>
[05:26:36.911] paramLI = 0x0
[05:26:36.911] estate = 0x0
[05:26:36.911] planstart = {ticks = <optimized out>}
[05:26:36.911] planduration = {ticks = 1103806595203}
[05:26:36.911] bufusage_start = {shared_blks_hit = 1,
shared_blks_read = 16443, shared_blks_dirtied = 16443,
shared_blks_written = 8, local_blks_hit = 94307489783240,
local_blks_read = 94307451894117, local_blks_dirtied = 94307489783240,
local_blks_written = 140727004487184, temp_blks_read = 0,
temp_blks_written = 94307489783416, shared_blk_read_time = {ticks =
0}, shared_blk_write_time = {ticks = 94307489780192},
local_blk_read_time = {ticks = 0}, local_blk_write_time = {ticks =
94307491025040}, temp_blk_read_time = {ticks = 0}, temp_blk_write_time
= {ticks = 0}}
[05:26:36.911] bufusage = {shared_blks_hit = 140727004486192,
shared_blks_read = 140061866319832, shared_blks_dirtied = 8,
shared_blks_written = 94307447196988, local_blks_hit = 34359738376,
local_blks_read = 94307489783240, local_blks_dirtied = 70622147264512,
local_blks_written = 94307491357144, temp_blks_read = 140061866319832,
temp_blks_written = 94307489783240, shared_blk_read_time = {ticks =
140727004486192}, shared_blk_write_time = {ticks = 140061866319832},
local_blk_read_time = {ticks = 8}, local_blk_write_time = {ticks =
94307489783240}, temp_blk_read_time = {ticks = 0}, temp_blk_write_time
= {ticks = 94307447197163}}
[05:26:36.911] revalidationResult = <optimized out>
[05:26:36.911] #7 0x000055c5a9daa387 in ExplainOneUtility
(utilityStmt=0x55c5ac6d9d38, into=into(at)entry=0x0,
es=es(at)entry=0x55c5ac859648,
queryString=queryString(at)entry=0x55c5ac6d91e0 "EXPLAIN (VERBOSE, COSTS
OFF) EXECUTE st6;", params=params(at)entry=0x0,
queryEnv=queryEnv(at)entry=0x0) at ../src/backend/commands/explain.c:495
[05:26:36.911] __func__ = "ExplainOneUtility"

[1] - https://cirrus-ci.com/task/5770112389611520?logs=cores#L71

Regards,
Vignesh