From 336ac6af015c97bfce8fbc51059001dd42e70a60 Mon Sep 17 00:00:00 2001 From: Rui Zhao Date: Mon, 28 Sep 2026 01:35:14 +0800 Subject: [PATCH] Fix cached-plan ownership during portal replanning A portal acquires its cached-plan reference with a NULL resource owner, and PortalReleaseCachedPlan() releases it the same way. Keep that ownership convention when PortalLockCachedPlan() releases an invalid plan and acquires its replacement. Executor locks still belong to the portal resource owner. Include the ownership correction posted by Manu, based on the diagnosis by Ilmar Yunusov, and add regression coverage for the retry. The existing invalidation case uses EXPLAIN EXECUTE, whose retry loop is in ExplainExecuteQuery(). Preserve that case and also exercise ordinary EXECUTE, which retries in PortalLockCachedPlan(). Run once without requesting DDL to cache a valid plan after the earlier index creation, then re-arm the DDL trigger and execute again. --- src/backend/tcop/pquery.c | 13 ++++++++++--- src/test/regress/expected/plancache.out | 16 ++++++++++++++++ src/test/regress/sql/plancache.sql | 6 ++++++ 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c index a81a45700c..a80cdf5e11 100644 --- a/src/backend/tcop/pquery.c +++ b/src/backend/tcop/pquery.c @@ -1863,13 +1863,20 @@ PortalLockCachedPlan(Portal portal, bool do_prep, else if (AcquireExecutorLocks(portal->cplan)) return false; - /* Replan. Locks will be taken freshly. */ - ReleaseCachedPlan(portal->cplan, portal->resowner); + /* + * Replan. Locks will be taken freshly. + * + * The portal manages its cached-plan reference directly, as in + * ExecuteQuery(), exec_bind_message(), and PortalReleaseCachedPlan(). + * Use a NULL resource owner for both release and acquisition here; + * executor locks still belong to portal->resowner. + */ + ReleaseCachedPlan(portal->cplan, NULL); portal->cplan = NULL; portal->stmts = NIL; portal->cplan = GetCachedPlan(portal->plansource, portal->portalParams, - portal->resowner, + NULL, portal->queryEnv); portal->stmts = portal->cplan->stmt_list; portal->strategy = ChoosePortalStrategy(portal->stmts); diff --git a/src/test/regress/expected/plancache.out b/src/test/regress/expected/plancache.out index 54077294dc..7048440ce6 100644 --- a/src/test/regress/expected/plancache.out +++ b/src/test/regress/expected/plancache.out @@ -461,6 +461,22 @@ NOTICE: creating index on partition inval_during_pruning_p1 Filter: (inval_during_pruning_p_1.a = stable_pruning_val()) (5 rows) +-- Cache a valid plan again after the preceding DDL. +execute inval_during_pruning_q; + a +--- + 1 +(1 row) + +-- Exercise PortalLockCachedPlan(), not just ExplainExecuteQuery(). +update inval_during_pruning_signal set create_idx = true; +execute inval_during_pruning_q; +NOTICE: creating index on partition inval_during_pruning_p1 + a +--- + 1 +(1 row) + deallocate inval_during_pruning_q; drop table inval_during_pruning_p, inval_during_pruning_signal; drop function invalidate_plancache_func, stable_pruning_val; diff --git a/src/test/regress/sql/plancache.sql b/src/test/regress/sql/plancache.sql index 90b6c5f82b..6daf7475d9 100644 --- a/src/test/regress/sql/plancache.sql +++ b/src/test/regress/sql/plancache.sql @@ -275,6 +275,12 @@ explain (verbose, costs off) execute inval_during_pruning_q; update inval_during_pruning_signal set create_idx = true; explain (verbose, costs off) execute inval_during_pruning_q; +-- Cache a valid plan again after the preceding DDL. +execute inval_during_pruning_q; +-- Exercise PortalLockCachedPlan(), not just ExplainExecuteQuery(). +update inval_during_pruning_signal set create_idx = true; +execute inval_during_pruning_q; + deallocate inval_during_pruning_q; drop table inval_during_pruning_p, inval_during_pruning_signal; drop function invalidate_plancache_func, stable_pruning_val; -- 2.43.7