On Mon, 31 Mar 2025 20:00:57 +0900 Yugo Nagata <nag...@sraoss.co.jp> wrote:
> Hi, > > I found that multiple sessions concurrently execute CREATE OR REPLACE FUNCTION > for a same function, the error "tuple concurrently updated" is raised. This is > an internal error output by elog, also the message is not user-friendly. > > I've attached a patch to prevent this internal error by locking an exclusive > lock before the command and get the read tuple after acquiring the lock. > Also, if the function has been removed during the lock waiting, the new entry > is created. I also found the same error is raised when concurrent ALTER FUNCTION commands are executed. I've added a patch to fix this in the similar way. Regards, Yugo Nagata -- Yugo Nagata <nag...@sraoss.co.jp>
>From e53f318dd42f909011227d3b11e8345ab7cc5641 Mon Sep 17 00:00:00 2001 From: Yugo Nagata <nag...@sraoss.co.jp> Date: Mon, 31 Mar 2025 20:17:07 +0900 Subject: [PATCH 2/2] Prevent internal error at concurrent ALTER FUNCTION --- src/backend/commands/functioncmds.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c index b9fd7683abb..3b9d9bb098c 100644 --- a/src/backend/commands/functioncmds.c +++ b/src/backend/commands/functioncmds.c @@ -61,6 +61,7 @@ #include "parser/parse_func.h" #include "parser/parse_type.h" #include "pgstat.h" +#include "storage/lmgr.h" #include "tcop/pquery.h" #include "tcop/utility.h" #include "utils/acl.h" @@ -1384,6 +1385,22 @@ AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt) if (!HeapTupleIsValid(tup)) /* should not happen */ elog(ERROR, "cache lookup failed for function %u", funcOid); + /* Lock the function so nobody else can do anything with it. */ + LockDatabaseObject(ProcedureRelationId, funcOid, 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. + */ + tup = SearchSysCacheCopy1(PROCOID, ObjectIdGetDatum(funcOid)); + if (!HeapTupleIsValid(tup)) + ereport(ERROR, + errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("function \"%s\" does not exist", + NameListToString(stmt->func->objname))); + procForm = (Form_pg_proc) GETSTRUCT(tup); /* Permission check: must own function */ -- 2.34.1
>From 49c890eb8cb115586e0e92294fb2fec60d80b8be Mon Sep 17 00:00:00 2001 From: Yugo Nagata <nag...@sraoss.co.jp> Date: Mon, 31 Mar 2025 18:46:58 +0900 Subject: [PATCH 1/2] Prevent internal error at concurrent CREATE OR REPLACE FUNCTION --- 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 fe0490259e9..cfd2e08ea23 100644 --- a/src/backend/catalog/pg_proc.c +++ b/src/backend/catalog/pg_proc.c @@ -35,6 +35,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" @@ -355,24 +356,45 @@ 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))); + + /* 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); @@ -553,11 +575,13 @@ ProcedureCreate(const char *procedureName, replaces[Anum_pg_proc_proowner - 1] = false; replaces[Anum_pg_proc_proacl - 1] = false; + + /* Okay, do it... */ 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.34.1