From 208f4bcc5b6f12ae95b2c2981786a441263b8a98 Mon Sep 17 00:00:00 2001 From: Yugo Nagata Date: Mon, 31 Mar 2025 18:46:58 +0900 Subject: [PATCH v5 1/3] Prevent internal error at concurrent CREATE OR REPLACE FUNCTION Previously, concurrent CREATE OR REPLACE FUNCTION commands could fail with an internal error "tuple concurrently updated". This occurred because multiple sessions attempted to modify the same catalog tuple simultaneously. To prevent this, ensure that an exclusive lock on the function object is acquired earlier in the process. Additionally, if the target function is dropped by another session while waiting for the lock, a new function is created instead. --- src/backend/catalog/pg_proc.c | 40 ++++++++++++++++++++++++++++------- 1 file changed, 32 insertions(+), 8 deletions(-) diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c index 5fdcf24d5f8..734508c7f58 100644 --- a/src/backend/catalog/pg_proc.c +++ b/src/backend/catalog/pg_proc.c @@ -34,6 +34,7 @@ #include "parser/parse_coerce.h" #include "pgstat.h" #include "rewrite/rewriteHandler.h" +#include "storage/lmgr.h" #include "tcop/pquery.h" #include "tcop/tcopprot.h" #include "utils/acl.h" @@ -383,24 +384,47 @@ ProcedureCreate(const char *procedureName, tupDesc = RelationGetDescr(rel); /* Check for pre-existing definition */ - oldtup = SearchSysCache3(PROCNAMEARGSNSP, - PointerGetDatum(procedureName), - PointerGetDatum(parameterTypes), - ObjectIdGetDatum(procNamespace)); + oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP, + PointerGetDatum(procedureName), + PointerGetDatum(parameterTypes), + ObjectIdGetDatum(procNamespace)); if (HeapTupleIsValid(oldtup)) { /* There is one; okay to replace it? */ Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup); - Datum proargnames; - bool isnull; - const char *dropcmd; if (!replace) ereport(ERROR, (errcode(ERRCODE_DUPLICATE_FUNCTION), errmsg("function \"%s\" already exists with same argument types", procedureName))); + + heap_freetuple(oldtup); + + /* Lock the function so nobody else can do anything with it. */ + LockDatabaseObject(ProcedureRelationId, oldproc->oid, 0, AccessExclusiveLock); + + /* + * It is possible that by the time we acquire the lock on function, + * concurrent DDL has removed it. We can test this by checking the + * existence of function. We get the tuple again to avoid the risk + * of function definition getting changed. + */ + oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP, + PointerGetDatum(procedureName), + PointerGetDatum(parameterTypes), + ObjectIdGetDatum(procNamespace)); + } + + if (HeapTupleIsValid(oldtup)) + { + + Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup); + Datum proargnames; + bool isnull; + const char *dropcmd; + if (!object_ownercheck(ProcedureRelationId, oldproc->oid, proowner)) aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_FUNCTION, procedureName); @@ -585,7 +609,7 @@ ProcedureCreate(const char *procedureName, tup = heap_modify_tuple(oldtup, tupDesc, values, nulls, replaces); CatalogTupleUpdate(rel, &tup->t_self, tup); - ReleaseSysCache(oldtup); + heap_freetuple(oldtup); is_update = true; } else -- 2.43.0