From 052ab8fe38493ca106d749f4e2426a86d0267d59 Mon Sep 17 00:00:00 2001 From: Amit Langote Date: Thu, 20 Nov 2025 15:35:47 +0900 Subject: [PATCH v3 5/6] Add test exercising prep cleanup on cached-plan invalidation Add a regression test that causes a generic plan to become invalid while pruning-aware setup is running. The pruning expression calls a function that can perform DDL on a partition, making the plan stale during reuse. The test's purpose is to drive execution through the invalidation path that discards any ExecutorPrep state created before the plan was found invalid, providing coverage for that cleanup logic. --- src/backend/utils/cache/plancache.c | 38 +++++++++++++-- src/test/regress/expected/plancache.out | 61 +++++++++++++++++++++++++ src/test/regress/sql/plancache.sql | 50 ++++++++++++++++++++ 3 files changed, 144 insertions(+), 5 deletions(-) diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c index c1cfd47422c..a9a4e11d1a5 100644 --- a/src/backend/utils/cache/plancache.c +++ b/src/backend/utils/cache/plancache.c @@ -103,6 +103,7 @@ static Query *QueryListGetPrimaryStmt(List *stmts); static void AcquireExecutorLocks(List *stmt_list, bool acquire); static void AcquireExecutorLocksUnpruned(List *stmt_list, bool acquire, CachedPlanPrepData *cprep); +static void CachedPlanPrepCleanup(CachedPlanPrepData *cprep); static void AcquirePlannerLocks(List *stmt_list, bool acquire); static void ScanQueryForLocks(Query *parsetree, bool acquire); static bool ScanQueryWalker(Node *node, bool *acquire); @@ -1033,6 +1034,9 @@ PrepAndCheckCachedPlan(CachedPlanSource *plansource, CachedPlanPrepData *cprep) /* Oops, the race case happened. Release useless locks. */ AcquireExecutorLocksWithPolicy(plan->stmt_list, policy, false, cprep); + + /* Also clean up ExecutorPrep() state, if necessary. */ + CachedPlanPrepCleanup(cprep); } /* @@ -2069,7 +2073,6 @@ LockRelids(List *rtable, Bitmapset *relids, bool acquire) * - looks up the ExecPrep object for each PlannedStmt from cprep->prep_list * (which must already be populated) * - unlocks the same relations identified during acquire - * - calls ExecPrepCleanup() on each ExecPrep * * prep_list is extended during acquire and must match stmt_list one-to-one * when releasing locks. Memory allocation for ExecPrep happens in @@ -2165,15 +2168,40 @@ AcquireExecutorLocksUnpruned(List *stmt_list, bool acquire, LockRelids(plannedstmt->rtable, lock_relids, acquire); bms_free(lock_relids); } - - /* Clean up prep if releasing locks. */ - if (!acquire) - ExecPrepCleanup(prep); } MemoryContextSwitchTo(oldcontext); } +/* + * CachedPlanPrepCleanup + * Clean up ExecPrep state built for a generic plan. + * + * This is used in the corner case where PrepAndCheckCachedPlan() discovers + * that a CachedPlan has become invalid after AcquireExecutorLocksUnpruned() + * has already run. In that case we must both release the execution locks + * and dispose of the ExecPrep list stored in CachedPlanPrepData, since the + * executor will never see or clean it up. + */ +static void +CachedPlanPrepCleanup(CachedPlanPrepData *cprep) +{ + ListCell *lc; + + if (cprep == NULL) + return; + + foreach(lc, cprep->prep_list) + { + ExecPrep *prep = (ExecPrep *) lfirst(lc); + + ExecPrepCleanup(prep); + } + + list_free(cprep->prep_list); + cprep->prep_list = NIL; +} + /* * AcquirePlannerLocks: acquire locks needed for planning of a querytree list; * or release them if acquire is false. diff --git a/src/test/regress/expected/plancache.out b/src/test/regress/expected/plancache.out index 4e59188196c..26c4c5e10fd 100644 --- a/src/test/regress/expected/plancache.out +++ b/src/test/regress/expected/plancache.out @@ -398,3 +398,64 @@ select name, generic_plans, custom_plans from pg_prepared_statements (1 row) drop table test_mode; +-- Test invalidation of a generic plan during pruning-aware lock setup. +-- The pruning expression uses a stable SQL function that calls a volatile +-- plpgsql function. That function performs DDL on a partition when a +-- separate "signal" table says to do so. The second EXECUTE should +-- replan cleanly after the DDL. +set plan_cache_mode to force_generic_plan; +create table inval_during_pruning_p (a int) partition by list (a); +create table inval_during_pruning_p1 partition of inval_during_pruning_p for values in (1); +create table inval_during_pruning_p2 partition of inval_during_pruning_p for values in (2); +insert into inval_during_pruning_p values (1), (2); +create table inval_during_pruning_signal (create_idx bool not null); +insert into inval_during_pruning_signal values (false); +create or replace function invalidate_plancache_func() returns int +as $$ +declare + create_index bool; +begin + -- Perform DDL on a partition if asked to + select create_idx into create_index from inval_during_pruning_signal for update; + if create_index = true then + raise notice 'creating index on partition inval_during_pruning_p1'; + create index on inval_during_pruning_p1 (a); + update inval_during_pruning_signal set create_idx = false; + end if; + -- pruning parameter + return 1; +end; +$$ language plpgsql volatile; +create or replace function stable_pruning_val() returns int as $$ + select invalidate_plancache_func(); +$$ language sql stable; +prepare inval_during_pruning_q as select * from inval_during_pruning_p where a = stable_pruning_val(); +-- Build a generic plan and run pruning once, but don't set the signal +-- for invalidate_plancache_func() to perform the DDL. +explain (verbose, costs off) execute inval_during_pruning_q; + QUERY PLAN +--------------------------------------------------------------------------- + Append + Subplans Removed: 1 + -> Seq Scan on public.inval_during_pruning_p1 inval_during_pruning_p_1 + Output: inval_during_pruning_p_1.a + Filter: (inval_during_pruning_p_1.a = stable_pruning_val()) +(5 rows) + +-- Reuse the generic plan. Make invalidate_plancache_func() perform DDL +-- during this execution, which should force replanning without errors. +update inval_during_pruning_signal set create_idx = true; +explain (verbose, costs off) execute inval_during_pruning_q; +NOTICE: creating index on partition inval_during_pruning_p1 + QUERY PLAN +--------------------------------------------------------------------------- + Append + Subplans Removed: 1 + -> Seq Scan on public.inval_during_pruning_p1 inval_during_pruning_p_1 + Output: inval_during_pruning_p_1.a + Filter: (inval_during_pruning_p_1.a = stable_pruning_val()) +(5 rows) + +drop table inval_during_pruning_p, inval_during_pruning_signal; +drop function invalidate_plancache_func, stable_pruning_val; +reset plan_cache_mode; diff --git a/src/test/regress/sql/plancache.sql b/src/test/regress/sql/plancache.sql index 4b2f11dcc64..cc7eb4da4d3 100644 --- a/src/test/regress/sql/plancache.sql +++ b/src/test/regress/sql/plancache.sql @@ -223,3 +223,53 @@ select name, generic_plans, custom_plans from pg_prepared_statements where name = 'test_mode_pp'; drop table test_mode; + +-- Test invalidation of a generic plan during pruning-aware lock setup. +-- The pruning expression uses a stable SQL function that calls a volatile +-- plpgsql function. That function performs DDL on a partition when a +-- separate "signal" table says to do so. The second EXECUTE should +-- replan cleanly after the DDL. +set plan_cache_mode to force_generic_plan; +create table inval_during_pruning_p (a int) partition by list (a); +create table inval_during_pruning_p1 partition of inval_during_pruning_p for values in (1); +create table inval_during_pruning_p2 partition of inval_during_pruning_p for values in (2); +insert into inval_during_pruning_p values (1), (2); + +create table inval_during_pruning_signal (create_idx bool not null); +insert into inval_during_pruning_signal values (false); +create or replace function invalidate_plancache_func() returns int +as $$ +declare + create_index bool; +begin + -- Perform DDL on a partition if asked to + select create_idx into create_index from inval_during_pruning_signal for update; + if create_index = true then + raise notice 'creating index on partition inval_during_pruning_p1'; + create index on inval_during_pruning_p1 (a); + update inval_during_pruning_signal set create_idx = false; + end if; + -- pruning parameter + return 1; +end; +$$ language plpgsql volatile; + +create or replace function stable_pruning_val() returns int as $$ + select invalidate_plancache_func(); +$$ language sql stable; + +prepare inval_during_pruning_q as select * from inval_during_pruning_p where a = stable_pruning_val(); + +-- Build a generic plan and run pruning once, but don't set the signal +-- for invalidate_plancache_func() to perform the DDL. +explain (verbose, costs off) execute inval_during_pruning_q; + +-- Reuse the generic plan. Make invalidate_plancache_func() perform DDL +-- during this execution, which should force replanning without errors. +update inval_during_pruning_signal set create_idx = true; +explain (verbose, costs off) execute inval_during_pruning_q; + +drop table inval_during_pruning_p, inval_during_pruning_signal; +drop function invalidate_plancache_func, stable_pruning_val; + +reset plan_cache_mode; -- 2.47.3