From ef1d05e0e277f1387f19fa897366f2baae057dbe Mon Sep 17 00:00:00 2001 From: Haibo Yan Date: Tue, 15 Sep 2026 20:21:32 -0700 Subject: [PATCH v2] Invalidate cached plans when the pg_proc candidate set changes Cached queries record the function, procedure or aggregate that name resolution selected, but resolution also depends on the set of pg_proc candidates visible under the active search_path. Adding a candidate, moving one to another name or schema, or changing which call shapes an existing one can match, can therefore make fresh parse analysis of unchanged SQL select a different object while a cached query keeps its original resolution. For example: CREATE FUNCTION f(bigint) ...; PREPARE q AS SELECT f(1); EXECUTE q; -- f(bigint) CREATE FUNCTION f(int) ...; SELECT f(1); -- f(int) EXECUTE q; -- f(bigint), but should be f(int) The existing machinery cannot detect this. plancache.c records a PROCOID dependency on the function that was selected, which catches later changes to that function but says nothing about candidates that did not exist when the query was analyzed; and when the selected function is a built-in, record_plan_function_dependency records nothing at all, so shadowing a built-in from an earlier schema goes unnoticed as well. Comparing the search_path does not help either, since the setting need not change: it is enough for a new candidate to appear in a schema that is already listed. Have the catalog code that makes such a change tell the plan cache about it, through a new CacheInvalidateProcCandidates(). That sends a broad invalidation of just the PROCNAMEARGSNSP syscache, rather than flushing all of pg_proc's catalog caches, and a plancache.c callback treats such a broad invalidation as a reason to discard cached query trees and re-run parse analysis. The signal is emitted from ProcedureCreate() when a new callable object is created, and when a replacement changes pronargdefaults or provariadic, since both affect which argument lists FuncnameGetCandidates synthesizes for a candidate; and from the generic rename and set-schema paths in alter.c. Tuple-specific pg_proc invalidations continue to use the existing precise mechanism. Replacing a function body, or altering cost, strictness, volatility, ownership or privileges, cannot move any candidate set, and still invalidates only the plans that recorded a PROCOID dependency on that particular function. No signal is sent for DROP either: removing the selected object is already covered by its PROCOID dependency, removing a candidate that lost cannot change an already-selected winner, and built-in winners cannot be dropped. Tests cover a candidate added to the same schema, a candidate added to an earlier schema of an unchanged search_path, and a CREATE OR REPLACE that adds a parameter default and thereby makes an existing call ambiguous. --- src/backend/catalog/pg_proc.c | 32 ++++++++++++ src/backend/commands/alter.c | 19 +++++++ src/backend/utils/cache/inval.c | 30 +++++++++++ src/backend/utils/cache/plancache.c | 45 +++++++++++++++- src/include/utils/inval.h | 2 + src/test/regress/expected/plancache.out | 68 +++++++++++++++++++++++++ src/test/regress/sql/plancache.sql | 45 ++++++++++++++++ 7 files changed, 240 insertions(+), 1 deletion(-) diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c index 53e27dd10e6..7cd6fd05337 100644 --- a/src/backend/catalog/pg_proc.c +++ b/src/backend/catalog/pg_proc.c @@ -39,6 +39,7 @@ #include "tcop/tcopprot.h" #include "utils/acl.h" #include "utils/builtins.h" +#include "utils/inval.h" #include "utils/lsyscache.h" #include "utils/regproc.h" #include "utils/rel.h" @@ -141,6 +142,8 @@ ProcedureCreate(const char *procedureName, NameData procname; TupleDesc tupDesc; bool is_update; + int16 old_pronargdefaults = 0; + Oid old_provariadic = InvalidOid; ObjectAddress myself, referenced, temp_object; @@ -398,6 +401,10 @@ ProcedureCreate(const char *procedureName, bool isnull; const char *dropcmd; + /* Remember the columns that determine candidate applicability */ + old_pronargdefaults = oldproc->pronargdefaults; + old_provariadic = oldproc->provariadic; + if (!replace) ereport(ERROR, (errcode(ERRCODE_DUPLICATE_FUNCTION), @@ -614,6 +621,31 @@ ProcedureCreate(const char *procedureName, retval = ((Form_pg_proc) GETSTRUCT(tup))->oid; + /* + * If we created a new callable object, or changed which call shapes an + * existing one can match, function name resolution may now come out + * differently for SQL that was analyzed earlier. Both parameter defaults + * and VARIADIC affect the argument lists that FuncnameGetCandidates + * synthesizes for a candidate, so a change to either one matters even + * though the object's OID and signature stay the same. + * + * proname, proargtypes and pronamespace cannot change here: we located + * oldtup by an exact match on those columns, so altering any of them + * creates a different object instead. Other in-place changes, such as the + * body or the volatility, cannot affect any candidate set, and plancache.c + * tracks them precisely by way of the selected function's OID. + */ + if (!is_update) + CacheInvalidateProcCandidates(); + else + { + Form_pg_proc newproc = (Form_pg_proc) GETSTRUCT(tup); + + if (newproc->pronargdefaults != old_pronargdefaults || + newproc->provariadic != old_provariadic) + CacheInvalidateProcCandidates(); + } + /* * Create dependencies for the new function. If we are updating an * existing function, first delete any existing pg_depend entries. diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c index 3f2af57167c..65dc1d2e7ff 100644 --- a/src/backend/commands/alter.c +++ b/src/backend/commands/alter.c @@ -62,6 +62,7 @@ #include "storage/lmgr.h" #include "utils/acl.h" #include "utils/builtins.h" +#include "utils/inval.h" #include "utils/lsyscache.h" #include "utils/rel.h" #include "utils/syscache.h" @@ -347,6 +348,16 @@ AlterObjectRename_internal(Relation rel, Oid objectId, const char *new_name) */ InvalidatePubRelSyncCache(pub->oid, pub->puballtables); } + else if (classId == ProcedureRelationId) + { + /* + * Renaming a callable object changes the candidate sets of both the + * old and the new name, so previously analyzed SQL may now resolve + * differently. Its OID does not change, so the plan cache's + * dependencies on the previously selected function would not notice. + */ + CacheInvalidateProcCandidates(); + } /* Release memory */ pfree(values); @@ -807,6 +818,14 @@ AlterObjectNamespace_internal(Relation rel, Oid objid, Oid nspOid) /* Perform actual update */ CatalogTupleUpdate(rel, &tup->t_self, newtup); + /* + * Moving a callable object to another schema changes which names it is + * visible under, and with what search_path precedence; compare the rename + * case in AlterObjectRename_internal. + */ + if (classId == ProcedureRelationId) + CacheInvalidateProcCandidates(); + /* Release memory */ pfree(values); pfree(nulls); diff --git a/src/backend/utils/cache/inval.c b/src/backend/utils/cache/inval.c index 81a5d433bc7..654d3f4753d 100644 --- a/src/backend/utils/cache/inval.c +++ b/src/backend/utils/cache/inval.c @@ -1591,6 +1591,36 @@ CacheInvalidateHeapTupleInplace(Relation relation, PrepareInplaceInvalidationState); } +/* + * CacheInvalidateProcCandidates + * Register an invalidation event for cached function resolution results. + * + * Cached analyzed queries record the function, procedure or aggregate that + * name resolution selected, but resolution also depends on the set of pg_proc + * candidates visible under the active search_path. Adding a candidate, moving + * one to a different name or schema, or changing which call shapes an existing + * one can match, can all make fresh parse analysis of unchanged SQL select a + * different object, without modifying the previously selected one. Callers + * that make such a change must call this, so that cached queries get + * reanalyzed. + * + * There is nothing object-specific to report here, since the affected queries + * need not mention any particular function, so we send a broad invalidation of + * just the PROCNAMEARGSNSP syscache. That is deliberately not + * CacheInvalidateCatalog(ProcedureRelationId), which would flush every pg_proc + * catalog cache in every backend; those cached tuples remain valid, only + * results derived from them during parse analysis do not. + */ +void +CacheInvalidateProcCandidates(void) +{ + if (IsBootstrapProcessingMode()) + return; + + RegisterCatcacheInvalidation(PROCNAMEARGSNSP, 0, MyDatabaseId, + PrepareInvalidationState()); +} + /* * CacheInvalidateCatalog * Register invalidation of the whole content of a system catalog. diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c index a1b406cee29..e015b362ff7 100644 --- a/src/backend/utils/cache/plancache.c +++ b/src/backend/utils/cache/plancache.c @@ -36,7 +36,13 @@ * certain other system catalogs, such as pg_namespace; but for them, our * response is just to invalidate all plans. We expect updates on those * catalogs to be infrequent enough that more-detailed tracking is not worth - * the effort. We likewise watch pg_authid, pg_auth_members, and + * the effort. Function name resolution needs special treatment: it depends on + * the whole set of pg_proc candidates visible under the active search_path, + * not only on the function that resolution selected, so the pg_proc + * dependencies described above cannot detect a change to it. Catalog changes + * that can move a candidate set send a separate broad invalidation instead + * (see CacheInvalidateProcCandidates), and we respond by invalidating all + * plans. We likewise watch pg_authid, pg_auth_members, and * pg_database, which can change which row-level security policies apply. * Since those are shared catalogs whose inval events reach every backend * in the cluster, we invalidate only the role-dependent plans. @@ -116,6 +122,9 @@ static void PlanCacheRoleCallback(Datum arg, SysCacheIdentifier cacheid, uint32 hashvalue); static void PlanCacheSysCallback(Datum arg, SysCacheIdentifier cacheid, uint32 hashvalue); +static void PlanCacheProcCandidateCallback(Datum arg, + SysCacheIdentifier cacheid, + uint32 hashvalue); /* ResourceOwner callbacks to track plancache references */ static void ResOwnerReleaseCachedPlan(Datum res); @@ -155,6 +164,8 @@ InitPlanCache(void) { CacheRegisterRelcacheCallback(PlanCacheRelCallback, (Datum) 0); CacheRegisterSyscacheCallback(PROCOID, PlanCacheObjectCallback, (Datum) 0); + CacheRegisterSyscacheCallback(PROCNAMEARGSNSP, + PlanCacheProcCandidateCallback, (Datum) 0); CacheRegisterSyscacheCallback(TYPEOID, PlanCacheObjectCallback, (Datum) 0); CacheRegisterSyscacheCallback(NAMESPACEOID, PlanCacheSysCallback, (Datum) 0); CacheRegisterSyscacheCallback(OPEROID, PlanCacheSysCallback, (Datum) 0); @@ -2364,6 +2375,38 @@ PlanCacheRoleCallback(Datum arg, SysCacheIdentifier cacheid, uint32 hashvalue) } } +/* + * PlanCacheProcCandidateCallback + * Syscache inval callback function for PROCNAMEARGSNSP cache + * + * Cached queries record the function that name resolution selected, but + * resolution also depends on the whole set of pg_proc candidates visible under + * the active search_path (see FuncnameGetCandidates and func_get_detail). + * That dependency can't be expressed as a PlanInvalItem: a candidate added + * after parse analysis can't be named by a dependency recorded before it + * existed, and the selected function may not be recorded at all, since + * record_plan_function_dependency ignores built-in functions. + * + * Invalidations of individual pg_proc tuples are reported here too, because + * catcache invalidation follows tuple changes rather than key changes, but + * those represent body or property updates that cannot move any candidate + * set; PlanCacheObjectCallback already handles them precisely by way of + * PROCOID, so ignore them. A whole-cache invalidation, reported with + * hashvalue zero, does mean that resolution might now come out differently, + * so discard all cached query trees to force reanalysis. + * CacheInvalidateProcCandidates sends one for exactly that reason, but it is + * not the only source: other whole-cache resets, such as recovery from sinval + * queue overflow, also arrive this way, and reanalyzing more than strictly + * necessary then is harmless. + */ +static void +PlanCacheProcCandidateCallback(Datum arg, SysCacheIdentifier cacheid, + uint32 hashvalue) +{ + if (hashvalue == 0) + ResetPlanCache(); +} + /* * PlanCacheSysCallback * Syscache inval callback function for other caches diff --git a/src/include/utils/inval.h b/src/include/utils/inval.h index 735e42f7310..d962300643c 100644 --- a/src/include/utils/inval.h +++ b/src/include/utils/inval.h @@ -67,6 +67,8 @@ extern void CacheInvalidateHeapTupleInplace(Relation relation, extern void CacheInvalidateCatalog(Oid catalogId); +extern void CacheInvalidateProcCandidates(void); + extern void CacheInvalidateRelcache(Relation relation); extern void CacheInvalidateRelcacheAll(void); diff --git a/src/test/regress/expected/plancache.out b/src/test/regress/expected/plancache.out index d58534ca1cd..bd6f45b1f8a 100644 --- a/src/test/regress/expected/plancache.out +++ b/src/test/regress/expected/plancache.out @@ -402,3 +402,71 @@ select name, generic_plans, custom_plans from pg_prepared_statements (1 row) drop table test_mode; +-- Cached queries record the function that name resolution selected, but +-- resolution also depends on the set of visible pg_proc candidates, so a +-- change to that set must force reanalysis. +-- A candidate added to the same schema can outrank the one already selected. +create schema pc_ov; +set search_path = pc_ov, pg_catalog; +create function pc_f(bigint) returns text + language sql as $$ select 'bigint' $$; +prepare pc_q as select pc_f(1); +execute pc_q; + pc_f +-------- + bigint +(1 row) + +create function pc_f(int) returns text language sql as $$ select 'int' $$; +execute pc_q; + pc_f +------ + int +(1 row) + +-- A candidate added to an earlier schema of an unchanged search_path shadows +-- the one already selected. Both schemas exist before the PREPARE, so this +-- is not masked by a search_path change or by pg_namespace invalidation. +create schema pc_s1; +create schema pc_s2; +set search_path = pc_s2, pc_s1, pg_catalog; +create function pc_s1.pc_g(int) returns text language sql as $$ select 's1' $$; +prepare pc_q2(int) as select pc_g($1); +execute pc_q2(1); + pc_g +------ + s1 +(1 row) + +create function pc_s2.pc_g(int) returns text language sql as $$ select 's2' $$; +execute pc_q2(1); + pc_g +------ + s2 +(1 row) + +-- Giving an existing function a parameter default changes which call arities +-- it is a candidate for, although its OID and signature do not change. +set search_path = pc_ov, pg_catalog; +create function pc_h(int) returns text language sql as $$ select 'h1' $$; +create function pc_h(int, int) returns text language sql as $$ select 'h2' $$; +prepare pc_q3 as select pc_h(1); +execute pc_q3; + pc_h +------ + h1 +(1 row) + +create or replace function pc_h(int, int default 0) returns text + language sql as $$ select 'h2' $$; +execute pc_q3; +ERROR: function pc_h(integer) is not unique +DETAIL: Could not choose a best candidate function. +HINT: You might need to add explicit type casts. +deallocate pc_q; +deallocate pc_q2; +deallocate pc_q3; +drop function pc_f(bigint), pc_f(int), pc_h(int), pc_h(int, int); +drop function pc_s1.pc_g(int), pc_s2.pc_g(int); +reset search_path; +drop schema pc_ov, pc_s1, pc_s2; diff --git a/src/test/regress/sql/plancache.sql b/src/test/regress/sql/plancache.sql index aed388d03a1..f2c9a255f96 100644 --- a/src/test/regress/sql/plancache.sql +++ b/src/test/regress/sql/plancache.sql @@ -228,3 +228,48 @@ select name, generic_plans, custom_plans from pg_prepared_statements where name = 'test_mode_pp'; drop table test_mode; + +-- Cached queries record the function that name resolution selected, but +-- resolution also depends on the set of visible pg_proc candidates, so a +-- change to that set must force reanalysis. + +-- A candidate added to the same schema can outrank the one already selected. +create schema pc_ov; +set search_path = pc_ov, pg_catalog; +create function pc_f(bigint) returns text + language sql as $$ select 'bigint' $$; +prepare pc_q as select pc_f(1); +execute pc_q; +create function pc_f(int) returns text language sql as $$ select 'int' $$; +execute pc_q; + +-- A candidate added to an earlier schema of an unchanged search_path shadows +-- the one already selected. Both schemas exist before the PREPARE, so this +-- is not masked by a search_path change or by pg_namespace invalidation. +create schema pc_s1; +create schema pc_s2; +set search_path = pc_s2, pc_s1, pg_catalog; +create function pc_s1.pc_g(int) returns text language sql as $$ select 's1' $$; +prepare pc_q2(int) as select pc_g($1); +execute pc_q2(1); +create function pc_s2.pc_g(int) returns text language sql as $$ select 's2' $$; +execute pc_q2(1); + +-- Giving an existing function a parameter default changes which call arities +-- it is a candidate for, although its OID and signature do not change. +set search_path = pc_ov, pg_catalog; +create function pc_h(int) returns text language sql as $$ select 'h1' $$; +create function pc_h(int, int) returns text language sql as $$ select 'h2' $$; +prepare pc_q3 as select pc_h(1); +execute pc_q3; +create or replace function pc_h(int, int default 0) returns text + language sql as $$ select 'h2' $$; +execute pc_q3; + +deallocate pc_q; +deallocate pc_q2; +deallocate pc_q3; +drop function pc_f(bigint), pc_f(int), pc_h(int), pc_h(int, int); +drop function pc_s1.pc_g(int), pc_s2.pc_g(int); +reset search_path; +drop schema pc_ov, pc_s1, pc_s2; -- 2.54.0