Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

Started by Yugo Nagata10 months ago29 messages
#1Yugo Nagata
nagata@sraoss.co.jp
1 attachment(s)

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.

Regards,
Yugo Nagata

--
Yugo Nagata <nagata@sraoss.co.jp>

Attachments:

0001-Prevent-internal-error-at-concurrent-CREATE-OR-REPLA.patchtext/x-diff; name=0001-Prevent-internal-error-at-concurrent-CREATE-OR-REPLA.patchDownload
From 49c890eb8cb115586e0e92294fb2fec60d80b8be Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
Date: Mon, 31 Mar 2025 18:46:58 +0900
Subject: [PATCH] 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

#2Yugo Nagata
nagata@sraoss.co.jp
In reply to: Yugo Nagata (#1)
2 attachment(s)
Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

On Mon, 31 Mar 2025 20:00:57 +0900
Yugo Nagata <nagata@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 <nagata@sraoss.co.jp>

Attachments:

0002-Prevent-internal-error-at-concurrent-ALTER-FUNCTION.patchtext/x-diff; name=0002-Prevent-internal-error-at-concurrent-ALTER-FUNCTION.patchDownload
From e53f318dd42f909011227d3b11e8345ab7cc5641 Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@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

0001-Prevent-internal-error-at-concurrent-CREATE-OR-REPLA.patchtext/x-diff; name=0001-Prevent-internal-error-at-concurrent-CREATE-OR-REPLA.patchDownload
From 49c890eb8cb115586e0e92294fb2fec60d80b8be Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@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

#3Jim Jones
jim.jones@uni-muenster.de
In reply to: Yugo Nagata (#2)
Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

Hi!

On 31.03.25 13:22, Yugo Nagata wrote:

On Mon, 31 Mar 2025 20:00:57 +0900
Yugo Nagata <nagata@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

I just briefly tested this patch and it seems to work as expected for
CREATE OF REPLACE FUNCTION:

-- Session 1 (t1):

postgres=# BEGIN;
BEGIN
postgres=*# CREATE OR REPLACE FUNCTION f1()
RETURNS INT LANGUAGE plpgsql AS
$$ BEGIN RETURN 1; END;$$;
CREATE FUNCTION

-- Session 2 (t2)

postgres=# CREATE OR REPLACE FUNCTION f1()
RETURNS INT LANGUAGE plpgsql AS
$$ BEGIN RETURN 2; END;$$;

(wait)

-- Session 3 (t3)

postgres=# CREATE OR REPLACE FUNCTION f1()
RETURNS INT LANGUAGE plpgsql AS
$$ BEGIN RETURN 3; END;$$;

(wait)

-- Session 4 (t4)

postgres=# CREATE OR REPLACE FUNCTION f1()
RETURNS INT LANGUAGE plpgsql AS
$$ BEGIN RETURN 4; END;$$;
CREATE FUNCTION

(wait)

-- Session 1 (t5)

postgres=*# END;
COMMIT

at this point Sessions 2, 3, and 4 were released with: CREATE FUNCTION

-- Session 1 (t6)

postgres=# \sf f1
CREATE OR REPLACE FUNCTION public.f1()
 RETURNS integer
 LANGUAGE plpgsql
AS $function$ BEGIN RETURN 4; END;$function$

So... it no longer shows the error message:

ERROR:  tuple concurrently updated

I did the same for ALTER FUNCTION but I was unable to reproduce the
error your reported. Could you provide your script?

Best regards, Jim

#4jian he
jian.universality@gmail.com
In reply to: Yugo Nagata (#2)
Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

On Mon, Mar 31, 2025 at 7:22 PM Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Mon, 31 Mar 2025 20:00:57 +0900
Yugo Nagata <nagata@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.

hi.

+ /* 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));

we already called LockDatabaseObject, concurrent DDL can
not do DROP FUNCTION or ALTER FUNCTION.
so no need to call SearchSysCacheCopy3 again?

@@ -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... */
no need to add these two new lines.
#5jian he
jian.universality@gmail.com
In reply to: jian he (#4)
1 attachment(s)
Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

On Thu, May 22, 2025 at 10:25 AM jian he <jian.universality@gmail.com> wrote:

hi.
earlier, i didn't check patch 0002.

i think in AlterFunction add
/* Lock the function so nobody else can do anything with it. */
LockDatabaseObject(ProcedureRelationId, funcOid, 0, AccessExclusiveLock);

right after
funcOid = LookupFuncWithArgs(stmt->objtype, stmt->func, false);

should be fine.

attached are some simple isolation tests for
CREATE OR REPLACE FUNCTION, ALTER FUNCTION, DROP FUNCTION.

Attachments:

v1-0001-isolation-tests-for-concurrent-change-FUNCTION-definition.patchtext/x-patch; charset=US-ASCII; name=v1-0001-isolation-tests-for-concurrent-change-FUNCTION-definition.patchDownload
From 040c5f739dbd4e5640ba40ae7c30b86d312bd004 Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Fri, 23 May 2025 10:33:39 +0800
Subject: [PATCH v1 1/1] isolation tests for concurrent change FUNCTION
 definition

discussion: https://postgr.es/m/20250331200057.00a62760966a821d484ea904@sraoss.co.jp
---
 .../isolation/expected/change-function.out    | 39 +++++++++++++++++++
 src/test/isolation/isolation_schedule         |  1 +
 src/test/isolation/specs/change-function.spec | 29 ++++++++++++++
 3 files changed, 69 insertions(+)
 create mode 100644 src/test/isolation/expected/change-function.out
 create mode 100644 src/test/isolation/specs/change-function.spec

diff --git a/src/test/isolation/expected/change-function.out b/src/test/isolation/expected/change-function.out
new file mode 100644
index 00000000000..812346f2fc1
--- /dev/null
+++ b/src/test/isolation/expected/change-function.out
@@ -0,0 +1,39 @@
+unused step name: r1
+Parsed test spec with 2 sessions
+
+starting permutation: b1 b2 create_f1 alter_f s1 c1 r2
+step b1: BEGIN;
+step b2: BEGIN;
+step create_f1: CREATE OR REPLACE FUNCTION f1() RETURNS INT LANGUAGE sql AS $$ SELECT 2;$$;
+step alter_f: ALTER FUNCTION f1() COST 71; <waiting ...>
+step s1: SELECT pg_get_functiondef(oid) FROM pg_proc pp WHERE proname = 'f1';
+pg_get_functiondef                                                                                      
+--------------------------------------------------------------------------------------------------------
+CREATE OR REPLACE FUNCTION public.f1()
+ RETURNS integer
+ LANGUAGE sql
+AS $function$ SELECT 2;$function$
+
+(1 row)
+
+step c1: COMMIT;
+step alter_f: <... completed>
+step r2: ROLLBACK;
+
+starting permutation: b1 b2 drop_f create_f1 c2 c1
+step b1: BEGIN;
+step b2: BEGIN;
+step drop_f: DROP FUNCTION f1;
+step create_f1: CREATE OR REPLACE FUNCTION f1() RETURNS INT LANGUAGE sql AS $$ SELECT 2;$$; <waiting ...>
+step c2: COMMIT;
+step create_f1: <... completed>
+step c1: COMMIT;
+
+starting permutation: b1 b2 create_f2 create_f1 c2 c1
+step b1: BEGIN;
+step b2: BEGIN;
+step create_f2: CREATE OR REPLACE FUNCTION f1() RETURNS INT LANGUAGE sql AS $$ SELECT 3;$$;
+step create_f1: CREATE OR REPLACE FUNCTION f1() RETURNS INT LANGUAGE sql AS $$ SELECT 2;$$; <waiting ...>
+step c2: COMMIT;
+step create_f1: <... completed>
+step c1: COMMIT;
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index e3c669a29c7..99e3ce780a7 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -116,3 +116,4 @@ test: serializable-parallel-2
 test: serializable-parallel-3
 test: matview-write-skew
 test: lock-nowait
+test: change-function
diff --git a/src/test/isolation/specs/change-function.spec b/src/test/isolation/specs/change-function.spec
new file mode 100644
index 00000000000..987e1ee7cf1
--- /dev/null
+++ b/src/test/isolation/specs/change-function.spec
@@ -0,0 +1,29 @@
+setup
+{
+    CREATE OR REPLACE FUNCTION f1() RETURNS INT LANGUAGE sql AS $$ SELECT 1;$$;
+}
+
+teardown
+{
+    DROP FUNCTION IF EXISTS f1;
+}
+
+session "s1"
+step b1         { BEGIN;}
+step create_f1   { CREATE OR REPLACE FUNCTION f1() RETURNS INT LANGUAGE sql AS $$ SELECT 2;$$; }
+step c1         { COMMIT; }
+step s1         { SELECT pg_get_functiondef(oid) FROM pg_proc pp WHERE proname = 'f1'; }
+step r1         { ROLLBACK; }
+
+session "s2"
+step b2         { BEGIN;}
+step alter_f    { ALTER FUNCTION f1() COST 71; }
+step create_f2   { CREATE OR REPLACE FUNCTION f1() RETURNS INT LANGUAGE sql AS $$ SELECT 3;$$; }
+step drop_f     { DROP FUNCTION f1; }
+step c2         { COMMIT; }
+step r2         { ROLLBACK; }
+
+# Basic effects
+permutation b1 b2 create_f1 alter_f s1 c1 r2
+permutation b1 b2 drop_f create_f1 c2 c1
+permutation b1 b2 create_f2 create_f1 c2 c1
-- 
2.34.1

#6Yugo Nagata
nagata@sraoss.co.jp
In reply to: Jim Jones (#3)
Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

On Tue, 20 May 2025 17:30:35 +0200
Jim Jones <jim.jones@uni-muenster.de> wrote:

I just briefly tested this patch and it seems to work as expected for
CREATE OF REPLACE FUNCTION:

Thank you for reviewing the patch!

I did the same for ALTER FUNCTION but I was unable to reproduce the
error your reported. Could you provide your script?

I can see the error when two concurrent transactions issue
"alter function f() immutable".

Regards,
Yugo Nagata

--
Yugo Nagata <nagata@sraoss.co.jp>

#7Yugo Nagata
nagata@sraoss.co.jp
In reply to: jian he (#4)
Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

Hi,

On Thu, 22 May 2025 10:25:58 +0800
jian he <jian.universality@gmail.com> wrote:

Thank you for looking into it.

+ /* 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));

we already called LockDatabaseObject, concurrent DDL can
not do DROP FUNCTION or ALTER FUNCTION.
so no need to call SearchSysCacheCopy3 again?

The function may be dropped *before* we call LockDatabaseObject.
SearchSysCacheCopy3 is called for check this.
Plese see AlterPublication() as a similar code example.

@@ -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... */
no need to add these two new lines.

I'll remove the lines. Thanks.

Regards,
Yugo Nagata

--
Yugo Nagata <nagata@sraoss.co.jp>

#8Yugo Nagata
nagata@sraoss.co.jp
In reply to: jian he (#5)
2 attachment(s)
Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

On Fri, 23 May 2025 10:37:42 +0800
jian he <jian.universality@gmail.com> wrote:

On Thu, May 22, 2025 at 10:25 AM jian he <jian.universality@gmail.com> wrote:

hi.
earlier, i didn't check patch 0002.

i think in AlterFunction add
/* Lock the function so nobody else can do anything with it. */
LockDatabaseObject(ProcedureRelationId, funcOid, 0, AccessExclusiveLock);

right after
funcOid = LookupFuncWithArgs(stmt->objtype, stmt->func, false);

should be fine.

Thank you. That makes sense because we can reduce redundant call of SearchSysCacheCopy1
and HeapTupleIsValid. I've attached a updated patch.

Regards,
Yugo Nagata

--
Yugo Nagata <nagata@sraoss.co.jp>

Attachments:

v2-0002-Prevent-internal-error-at-concurrent-ALTER-FUNCTI.patchtext/x-diff; name=v2-0002-Prevent-internal-error-at-concurrent-ALTER-FUNCTI.patchDownload
From b5b79dffb96760b47632cc9d325a034f5cc020e9 Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
Date: Mon, 31 Mar 2025 20:17:07 +0900
Subject: [PATCH v2 2/2] Prevent internal error at concurrent ALTER FUNCTION

---
 src/backend/commands/functioncmds.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index b9fd7683abb..a41eec9d48e 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"
@@ -1380,9 +1381,21 @@ AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt)
 
 	ObjectAddressSet(address, ProcedureRelationId, 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)) /* should not happen */
-		elog(ERROR, "cache lookup failed for function %u", 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);
 
-- 
2.43.0

v2-0001-Prevent-internal-error-at-concurrent-CREATE-OR-RE.patchtext/x-diff; name=v2-0001-Prevent-internal-error-at-concurrent-CREATE-OR-RE.patchDownload
From 094fd8d212e01634f74afcac8a9b8b0d26201813 Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
Date: Mon, 31 Mar 2025 18:46:58 +0900
Subject: [PATCH v2 1/2] Prevent internal error at concurrent CREATE OR REPLACE
 FUNCTION

---
 src/backend/catalog/pg_proc.c | 38 +++++++++++++++++++++++++++--------
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index fe0490259e9..3a15374b261 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);
@@ -557,7 +579,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

#9jian he
jian.universality@gmail.com
In reply to: Yugo Nagata (#7)
Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

On Tue, May 27, 2025 at 1:35 AM Yugo Nagata <nagata@sraoss.co.jp> wrote:

+ /* 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));

we already called LockDatabaseObject, concurrent DDL can
not do DROP FUNCTION or ALTER FUNCTION.
so no need to call SearchSysCacheCopy3 again?

The function may be dropped *before* we call LockDatabaseObject.
SearchSysCacheCopy3 is called for check this.
Plese see AlterPublication() as a similar code example.

I am wondering, can we do it the following way for v2-0001?

/* Check for pre-existing definition */
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);
if (!replace)
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_FUNCTION),
errmsg("function \"%s\" already exists with same
argument types",
procedureName)));
/*
* 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.
*/
if (!ConditionalLockDatabaseObject(ProcedureRelationId,
oldproc->oid, 0, AccessExclusiveLock))
oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
PointerGetDatum(procedureName),
PointerGetDatum(parameterTypes),
ObjectIdGetDatum(procNamespace));
}

#10Yugo Nagata
nagata@sraoss.co.jp
In reply to: jian he (#9)
Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

On Tue, 27 May 2025 10:03:58 +0800
jian he <jian.universality@gmail.com> wrote:

On Tue, May 27, 2025 at 1:35 AM Yugo Nagata <nagata@sraoss.co.jp> wrote:

+ /* 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));

we already called LockDatabaseObject, concurrent DDL can
not do DROP FUNCTION or ALTER FUNCTION.
so no need to call SearchSysCacheCopy3 again?

The function may be dropped *before* we call LockDatabaseObject.
SearchSysCacheCopy3 is called for check this.
Plese see AlterPublication() as a similar code example.

I am wondering, can we do it the following way for v2-0001?

/* Check for pre-existing definition */
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);
if (!replace)
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_FUNCTION),
errmsg("function \"%s\" already exists with same
argument types",
procedureName)));
/*
* 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.
*/
if (!ConditionalLockDatabaseObject(ProcedureRelationId,
oldproc->oid, 0, AccessExclusiveLock))
oldtup = SearchSysCacheCopy3(PROCNAMEARGSNSP,
PointerGetDatum(procedureName),
PointerGetDatum(parameterTypes),
ObjectIdGetDatum(procNamespace));
}

No. This cannot prevent the error "ERROR: tuple concurrently updated"
because it doesn't wait for end of the concurrently running session
if the lock cannot be acquired.

Regards,
Yugo Nagata

--
Yugo Nagata <nagata@sraoss.co.jp>

#11Alexander Lakhin
exclusion@gmail.com
In reply to: Yugo Nagata (#2)
Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

Hello Yugo,

31.03.2025 14:22, Yugo Nagata wrote:

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 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.

FWIW, the same error is raised also with concurrent GRANT/REVOKE on a
database:
/messages/by-id/18dcfb7f-5deb-4487-ae22-a2c16839519a@gmail.com

Maybe you would also find relevant this thread:
/messages/by-id/ZiYjn0eVc7pxVY45@ip-10-97-1-34.eu-west-3.compute.internal

Best regards,
Alexander Lakhin
Neon (https://neon.tech)

#12Jim Jones
jim.jones@uni-muenster.de
In reply to: Yugo Nagata (#6)
Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

Hi Yugo

On 26.05.25 18:39, Yugo Nagata wrote:

I can see the error when two concurrent transactions issue
"alter function f() immutable".

I might have missed something in my last tests... I could now reproduce
the behaviour you mentioned.

I've tested v2 and it works as described. CREATE OR REPLACE FUNCTION and
ALTER TABLE no longer raise an error after the lock by the concurrent
transaction was freed.

One quick question in v2-002:

     tup = SearchSysCacheCopy1(PROCOID, ObjectIdGetDatum(funcOid));
-    if (!HeapTupleIsValid(tup)) /* should not happen */
-        elog(ERROR, "cache lookup failed for function %u", funcOid);
+    if (!HeapTupleIsValid(tup))
+        ereport(ERROR,
+                errcode(ERRCODE_UNDEFINED_OBJECT),
+                errmsg("function \"%s\" does not exist",
+                       NameListToString(stmt->func->objname)));

Is it really ok to change this error message here? Did the addition of
LockDatabaseObject change the semantics of the previous message? Other
similar parts of the code still report "cache lookup failed for function
x". I don't have a strong opinion here, but perhaps we should keep these
messages consistent at least throughout the file?

Thanks!

Best, Jim

#13Yugo Nagata
nagata@sraoss.co.jp
In reply to: Jim Jones (#12)
Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

On Tue, 27 May 2025 08:33:42 +0200
Jim Jones <jim.jones@uni-muenster.de> wrote:

Hi Yugo

On 26.05.25 18:39, Yugo Nagata wrote:

I can see the error when two concurrent transactions issue
"alter function f() immutable".

I might have missed something in my last tests... I could now reproduce
the behaviour you mentioned.

I've tested v2 and it works as described. CREATE OR REPLACE FUNCTION and
ALTER TABLE no longer raise an error after the lock by the concurrent
transaction was freed.

One quick question in v2-002:

     tup = SearchSysCacheCopy1(PROCOID, ObjectIdGetDatum(funcOid));
-    if (!HeapTupleIsValid(tup)) /* should not happen */
-        elog(ERROR, "cache lookup failed for function %u", funcOid);
+    if (!HeapTupleIsValid(tup))
+        ereport(ERROR,
+                errcode(ERRCODE_UNDEFINED_OBJECT),
+                errmsg("function \"%s\" does not exist",
+                       NameListToString(stmt->func->objname)));

Is it really ok to change this error message here? Did the addition of
LockDatabaseObject change the semantics of the previous message?

Yes. AcceptInvalidationMessages() is called in LockDatabaseObject() after wait,
and this enables the detection of object deletion during the wait.

Regards,
Yugo Nagata

--
Yugo Nagata <nagata@sraoss.co.jp>

#14Yugo Nagata
nagata@sraoss.co.jp
In reply to: Alexander Lakhin (#11)
Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

On Tue, 27 May 2025 09:00:01 +0300
Alexander Lakhin <exclusion@gmail.com> wrote:

Hello Yugo,

31.03.2025 14:22, Yugo Nagata wrote:

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 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.

FWIW, the same error is raised also with concurrent GRANT/REVOKE on a
database:
/messages/by-id/18dcfb7f-5deb-4487-ae22-a2c16839519a@gmail.com

Maybe you would also find relevant this thread:
/messages/by-id/ZiYjn0eVc7pxVY45@ip-10-97-1-34.eu-west-3.compute.internal

Thank you for sharing the information.

I know there are other scenarios where the same is raises and I agree that
it would be better to consider a more global solution instead of addressing
each of them. However, I am not sure that improving the error message for
each case doesn't not make sense.

Regards,
Yugo Nagata

--
Yugo Nagata <nagata@sraoss.co.jp>

#15Yugo Nagata
nagata@sraoss.co.jp
In reply to: Yugo Nagata (#14)
3 attachment(s)
Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

On Tue, 3 Jun 2025 17:39:50 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Tue, 27 May 2025 09:00:01 +0300
Alexander Lakhin <exclusion@gmail.com> wrote:

I know there are other scenarios where the same is raises and I agree that
it would be better to consider a more global solution instead of addressing
each of them. However, I am not sure that improving the error message for
each case doesn't not make sense.

To address the remaining cases where DDL commands fail with the internal
error "ERROR: tuple concurrently updated" due to insufficient locking,
I would like to propose improving the error reporting to produce a more
appropriate and user-facing error message. This should make it easier for
users to understand the cause of the failure.

Patch 0003 improves the error message shown when concurrent updates to a
system catalog tuple occur, producing output like:

ERROR: operation failed due to a concurrent command
DETAIL: Another command modified the same object in a concurrent session.

Patches 0001 and 0002 are unchanged from v2, except for updated commit messages.
I believe these patches are still useful, as they allow the operation to complete
successfully after waiting, or to behave appropriately when the target function
is dropped by another session during the wait.

Best regards,
Yugo Nagata

--
Yugo Nagata <nagata@sraoss.co.jp>

Attachments:

v3-0003-Improve-error-reporting-for-concurrent-updates-on.patchtext/x-diff; name=v3-0003-Improve-error-reporting-for-concurrent-updates-on.patchDownload
From 4ab2d5f6b4651b6cdad47a170f53147bb7d8d5c2 Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
Date: Thu, 5 Jun 2025 15:32:37 +0900
Subject: [PATCH v3 3/3] Improve error reporting for concurrent updates on
 system catalog tuples

Previously, when multiple sessions attempted to modify the same system
catalog tuple concurrently due to insufficient locking, DDL commands could
fail with an internal error:

ERROR:  tuple concurrently updated

This commit improves the behavior by reporting a more appropriate and
user-facing error message in such cases, making it easier for users to
understand the cause of the failure.
---
 src/backend/access/heap/heapam.c | 34 ++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 0dcd6ee817e..712b7ded6d2 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -3194,6 +3194,7 @@ simple_heap_delete(Relation relation, ItemPointer tid)
 {
 	TM_Result	result;
 	TM_FailureData tmfd;
+	bool is_catalog = IsCatalogRelation(relation);
 
 	result = heap_delete(relation, tid,
 						 GetCurrentCommandId(true), InvalidSnapshot,
@@ -3211,11 +3212,23 @@ simple_heap_delete(Relation relation, ItemPointer tid)
 			break;
 
 		case TM_Updated:
-			elog(ERROR, "tuple concurrently updated");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command modified the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently updated");
 			break;
 
 		case TM_Deleted:
-			elog(ERROR, "tuple concurrently deleted");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command dropped the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently deleted");
 			break;
 
 		default:
@@ -4486,6 +4499,7 @@ simple_heap_update(Relation relation, ItemPointer otid, HeapTuple tup,
 	TM_Result	result;
 	TM_FailureData tmfd;
 	LockTupleMode lockmode;
+	bool is_catalog = IsCatalogRelation(relation);
 
 	result = heap_update(relation, otid, tup,
 						 GetCurrentCommandId(true), InvalidSnapshot,
@@ -4503,11 +4517,23 @@ simple_heap_update(Relation relation, ItemPointer otid, HeapTuple tup,
 			break;
 
 		case TM_Updated:
-			elog(ERROR, "tuple concurrently updated");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command modified the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently updated");
 			break;
 
 		case TM_Deleted:
-			elog(ERROR, "tuple concurrently deleted");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command dropped the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently deleted");
 			break;
 
 		default:
-- 
2.43.0

v3-0002-Prevent-internal-error-caused-by-concurrent-ALTER.patchtext/x-diff; name=v3-0002-Prevent-internal-error-caused-by-concurrent-ALTER.patchDownload
From 56070e0ab5a73cc090f79850af40b418ddf53645 Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
Date: Mon, 31 Mar 2025 20:17:07 +0900
Subject: [PATCH v3 2/3] Prevent internal error caused by concurrent ALTER
 FUNCTION

Previously, concurrent ALTER 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, an appropriate error is raised to indicate the
object no longer exists.
---
 src/backend/commands/functioncmds.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index 0335e982b31..5bb03153c0d 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"
@@ -1383,9 +1384,21 @@ AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt)
 
 	ObjectAddressSet(address, ProcedureRelationId, 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)) /* should not happen */
-		elog(ERROR, "cache lookup failed for function %u", 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);
 
-- 
2.43.0

v3-0001-Prevent-internal-error-at-concurrent-CREATE-OR-RE.patchtext/x-diff; name=v3-0001-Prevent-internal-error-at-concurrent-CREATE-OR-RE.patchDownload
From 6c11eca73b0b5b847edf5e1eb5840ee692b1e5fe Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
Date: Mon, 31 Mar 2025 18:46:58 +0900
Subject: [PATCH v3 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 | 38 +++++++++++++++++++++++++++--------
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index 5fdcf24d5f8..a35696480c7 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,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);
@@ -585,7 +607,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

#16Yugo Nagata
nagata@sraoss.co.jp
In reply to: Yugo Nagata (#15)
3 attachment(s)
Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

On Thu, 5 Jun 2025 16:26:08 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Tue, 3 Jun 2025 17:39:50 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Tue, 27 May 2025 09:00:01 +0300
Alexander Lakhin <exclusion@gmail.com> wrote:

I know there are other scenarios where the same is raises and I agree that
it would be better to consider a more global solution instead of addressing
each of them. However, I am not sure that improving the error message for
each case doesn't not make sense.

To address the remaining cases where DDL commands fail with the internal
error "ERROR: tuple concurrently updated" due to insufficient locking,
I would like to propose improving the error reporting to produce a more
appropriate and user-facing error message. This should make it easier for
users to understand the cause of the failure.

Patch 0003 improves the error message shown when concurrent updates to a
system catalog tuple occur, producing output like:

ERROR: operation failed due to a concurrent command
DETAIL: Another command modified the same object in a concurrent session.

Patches 0001 and 0002 are unchanged from v2, except for updated commit messages.
I believe these patches are still useful, as they allow the operation to complete
successfully after waiting, or to behave appropriately when the target function
is dropped by another session during the wait.

I found that the error "tuple concurrently updated" was expected as the results
of injection_points test , so I've fixed it so that the new message is expected
instead.

I've attached updated patches.

Best regards,
Yugo Nagata

--
Yugo Nagata <nagata@sraoss.co.jp>

Attachments:

v4-0003-Improve-error-reporting-for-concurrent-updates-on.patchtext/x-diff; name=v4-0003-Improve-error-reporting-for-concurrent-updates-on.patchDownload
From b7ba94bb6ae4e646114b559b31f52e4c02958b6c Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
Date: Thu, 5 Jun 2025 15:32:37 +0900
Subject: [PATCH v4 3/3] Improve error reporting for concurrent updates on
 system catalog tuples

Previously, when multiple sessions attempted to modify the same system
catalog tuple concurrently due to insufficient locking, DDL commands could
fail with an internal error:

ERROR:  tuple concurrently updated

This commit improves the behavior by reporting a more appropriate and
user-facing error message in such cases, making it easier for users to
understand the cause of the failure.
---
 src/backend/access/heap/heapam.c              | 34 ++++++++++++++++---
 .../expected/syscache-update-pruned.out       |  2 +-
 2 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 0dcd6ee817e..712b7ded6d2 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -3194,6 +3194,7 @@ simple_heap_delete(Relation relation, ItemPointer tid)
 {
 	TM_Result	result;
 	TM_FailureData tmfd;
+	bool is_catalog = IsCatalogRelation(relation);
 
 	result = heap_delete(relation, tid,
 						 GetCurrentCommandId(true), InvalidSnapshot,
@@ -3211,11 +3212,23 @@ simple_heap_delete(Relation relation, ItemPointer tid)
 			break;
 
 		case TM_Updated:
-			elog(ERROR, "tuple concurrently updated");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command modified the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently updated");
 			break;
 
 		case TM_Deleted:
-			elog(ERROR, "tuple concurrently deleted");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command dropped the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently deleted");
 			break;
 
 		default:
@@ -4486,6 +4499,7 @@ simple_heap_update(Relation relation, ItemPointer otid, HeapTuple tup,
 	TM_Result	result;
 	TM_FailureData tmfd;
 	LockTupleMode lockmode;
+	bool is_catalog = IsCatalogRelation(relation);
 
 	result = heap_update(relation, otid, tup,
 						 GetCurrentCommandId(true), InvalidSnapshot,
@@ -4503,11 +4517,23 @@ simple_heap_update(Relation relation, ItemPointer otid, HeapTuple tup,
 			break;
 
 		case TM_Updated:
-			elog(ERROR, "tuple concurrently updated");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command modified the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently updated");
 			break;
 
 		case TM_Deleted:
-			elog(ERROR, "tuple concurrently deleted");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command dropped the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently deleted");
 			break;
 
 		default:
diff --git a/src/test/modules/injection_points/expected/syscache-update-pruned.out b/src/test/modules/injection_points/expected/syscache-update-pruned.out
index a6a4e8db996..231545a6cbb 100644
--- a/src/test/modules/injection_points/expected/syscache-update-pruned.out
+++ b/src/test/modules/injection_points/expected/syscache-update-pruned.out
@@ -20,7 +20,7 @@ step wakegrant4:
 	SELECT FROM injection_points_wakeup('heap_update-before-pin');
  <waiting ...>
 step grant1: <... completed>
-ERROR:  tuple concurrently deleted
+ERROR:  operation failed due to a concurrent command
 step wakegrant4: <... completed>
 
 starting permutation: cachefill1 at2 waitprunable4 vac4 grant1 wakeinval4 mkrels4 wakegrant4
-- 
2.43.0

v4-0002-Prevent-internal-error-caused-by-concurrent-ALTER.patchtext/x-diff; name=v4-0002-Prevent-internal-error-caused-by-concurrent-ALTER.patchDownload
From 56070e0ab5a73cc090f79850af40b418ddf53645 Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
Date: Mon, 31 Mar 2025 20:17:07 +0900
Subject: [PATCH v4 2/3] Prevent internal error caused by concurrent ALTER
 FUNCTION

Previously, concurrent ALTER 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, an appropriate error is raised to indicate the
object no longer exists.
---
 src/backend/commands/functioncmds.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index 0335e982b31..5bb03153c0d 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"
@@ -1383,9 +1384,21 @@ AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt)
 
 	ObjectAddressSet(address, ProcedureRelationId, 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)) /* should not happen */
-		elog(ERROR, "cache lookup failed for function %u", 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);
 
-- 
2.43.0

v4-0001-Prevent-internal-error-at-concurrent-CREATE-OR-RE.patchtext/x-diff; name=v4-0001-Prevent-internal-error-at-concurrent-CREATE-OR-RE.patchDownload
From 6c11eca73b0b5b847edf5e1eb5840ee692b1e5fe Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
Date: Mon, 31 Mar 2025 18:46:58 +0900
Subject: [PATCH v4 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 | 38 +++++++++++++++++++++++++++--------
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index 5fdcf24d5f8..a35696480c7 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,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);
@@ -585,7 +607,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

#17Daniil Davydov
3danissimo@gmail.com
In reply to: Yugo Nagata (#16)
Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

Hi,

On Thu, Jun 5, 2025 at 5:21 PM Yugo Nagata <nagata@sraoss.co.jp> wrote:

I've attached updated patches.

I have some comments on v4-0001 patch :
1)
heap_freetuple should be called for every tuple that we get from
SearchSysCacheCopy3.
But if tuple is valid after the first SearchSysCacheCopy3, we
overwrite the old pointer (by the second SearchSysCacheCopy3 call) and
forget to free it.
I suggest adding heap_freetuple call before the second SearchSysCacheCopy3 call.

2)
+        Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
+        Datum           proargnames;
+        bool            isnull;
+        const char *dropcmd;
Strange alignment. I guess you should keep the same alignment as in
deleted declarations.

3)
This patch fixes postgres behavior if I first create a function and
then try to CREATE OR REPLACE it in concurrent transactions.
But if the function doesn't exist and I try to call CREATE OR REPLACE
in concurrent transactions, I will get an error.
I wrote about it in this thread [1]/messages/by-id/CAJDiXghv2JF5zbLyyybokWKM+-GYsTG+hw7xseLNgJOJwf0+8w@mail.gmail.com and Tom Lane said that this
behavior is kinda expected.
Just in case, I decided to mention it here anyway - perhaps you will
have other thoughts on this matter.

[1]: /messages/by-id/CAJDiXghv2JF5zbLyyybokWKM+-GYsTG+hw7xseLNgJOJwf0+8w@mail.gmail.com

--
Best regards,
Daniil Davydov

#18Yugo Nagata
nagata@sraoss.co.jp
In reply to: Daniil Davydov (#17)
3 attachment(s)
Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

On Fri, 27 Jun 2025 18:53:02 +0700
Daniil Davydov <3danissimo@gmail.com> wrote:

Hi,

On Thu, Jun 5, 2025 at 5:21 PM Yugo Nagata <nagata@sraoss.co.jp> wrote:

I've attached updated patches.

I have some comments on v4-0001 patch :

Thank you for your comments!

1)
heap_freetuple should be called for every tuple that we get from
SearchSysCacheCopy3.
But if tuple is valid after the first SearchSysCacheCopy3, we
overwrite the old pointer (by the second SearchSysCacheCopy3 call) and
forget to free it.
I suggest adding heap_freetuple call before the second SearchSysCacheCopy3 call.

Good catches. Fixed.

2)
+        Form_pg_proc oldproc = (Form_pg_proc) GETSTRUCT(oldtup);
+        Datum           proargnames;
+        bool            isnull;
+        const char *dropcmd;
Strange alignment. I guess you should keep the same alignment as in
deleted declarations.

Fixed.

I've attached patches including these fixes.

3)
This patch fixes postgres behavior if I first create a function and
then try to CREATE OR REPLACE it in concurrent transactions.
But if the function doesn't exist and I try to call CREATE OR REPLACE
in concurrent transactions, I will get an error.
I wrote about it in this thread [1] and Tom Lane said that this
behavior is kinda expected.
Just in case, I decided to mention it here anyway - perhaps you will
have other thoughts on this matter.

[1] /messages/by-id/CAJDiXghv2JF5zbLyyybokWKM+-GYsTG+hw7xseLNgJOJwf0+8w@mail.gmail.com

I agree with Tom Lane that the behavior is expected, although it would be better
if the error message were more user-friendly. We could improve it by checking the
unique constraint before calling index_insert in CatalogIndexInsert.

Regards,
Yugo Nagata

--
Yugo Nagata <nagata@sraoss.co.jp>

Attachments:

v5-0003-Improve-error-reporting-for-concurrent-updates-on.patchtext/x-diff; name=v5-0003-Improve-error-reporting-for-concurrent-updates-on.patchDownload
From 7c9146f221a253aa3a150f6b6d5600ed0225e4e1 Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
Date: Thu, 5 Jun 2025 15:32:37 +0900
Subject: [PATCH v5 3/3] Improve error reporting for concurrent updates on
 system catalog tuples

Previously, when multiple sessions attempted to modify the same system
catalog tuple concurrently due to insufficient locking, DDL commands could
fail with an internal error:

ERROR:  tuple concurrently updated

This commit improves the behavior by reporting a more appropriate and
user-facing error message in such cases, making it easier for users to
understand the cause of the failure.
---
 src/backend/access/heap/heapam.c              | 34 ++++++++++++++++---
 .../expected/syscache-update-pruned.out       |  2 +-
 2 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 0dcd6ee817e..712b7ded6d2 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -3194,6 +3194,7 @@ simple_heap_delete(Relation relation, ItemPointer tid)
 {
 	TM_Result	result;
 	TM_FailureData tmfd;
+	bool is_catalog = IsCatalogRelation(relation);
 
 	result = heap_delete(relation, tid,
 						 GetCurrentCommandId(true), InvalidSnapshot,
@@ -3211,11 +3212,23 @@ simple_heap_delete(Relation relation, ItemPointer tid)
 			break;
 
 		case TM_Updated:
-			elog(ERROR, "tuple concurrently updated");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command modified the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently updated");
 			break;
 
 		case TM_Deleted:
-			elog(ERROR, "tuple concurrently deleted");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command dropped the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently deleted");
 			break;
 
 		default:
@@ -4486,6 +4499,7 @@ simple_heap_update(Relation relation, ItemPointer otid, HeapTuple tup,
 	TM_Result	result;
 	TM_FailureData tmfd;
 	LockTupleMode lockmode;
+	bool is_catalog = IsCatalogRelation(relation);
 
 	result = heap_update(relation, otid, tup,
 						 GetCurrentCommandId(true), InvalidSnapshot,
@@ -4503,11 +4517,23 @@ simple_heap_update(Relation relation, ItemPointer otid, HeapTuple tup,
 			break;
 
 		case TM_Updated:
-			elog(ERROR, "tuple concurrently updated");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command modified the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently updated");
 			break;
 
 		case TM_Deleted:
-			elog(ERROR, "tuple concurrently deleted");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command dropped the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently deleted");
 			break;
 
 		default:
diff --git a/src/test/modules/injection_points/expected/syscache-update-pruned.out b/src/test/modules/injection_points/expected/syscache-update-pruned.out
index a6a4e8db996..231545a6cbb 100644
--- a/src/test/modules/injection_points/expected/syscache-update-pruned.out
+++ b/src/test/modules/injection_points/expected/syscache-update-pruned.out
@@ -20,7 +20,7 @@ step wakegrant4:
 	SELECT FROM injection_points_wakeup('heap_update-before-pin');
  <waiting ...>
 step grant1: <... completed>
-ERROR:  tuple concurrently deleted
+ERROR:  operation failed due to a concurrent command
 step wakegrant4: <... completed>
 
 starting permutation: cachefill1 at2 waitprunable4 vac4 grant1 wakeinval4 mkrels4 wakegrant4
-- 
2.43.0

v5-0002-Prevent-internal-error-caused-by-concurrent-ALTER.patchtext/x-diff; name=v5-0002-Prevent-internal-error-caused-by-concurrent-ALTER.patchDownload
From 8e5aa491ee6754d55001f568557454fcfd65eb41 Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
Date: Mon, 31 Mar 2025 20:17:07 +0900
Subject: [PATCH v5 2/3] Prevent internal error caused by concurrent ALTER
 FUNCTION

Previously, concurrent ALTER 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, an appropriate error is raised to indicate the
object no longer exists.
---
 src/backend/commands/functioncmds.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index 0335e982b31..5bb03153c0d 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"
@@ -1383,9 +1384,21 @@ AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt)
 
 	ObjectAddressSet(address, ProcedureRelationId, 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)) /* should not happen */
-		elog(ERROR, "cache lookup failed for function %u", 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);
 
-- 
2.43.0

v5-0001-Prevent-internal-error-at-concurrent-CREATE-OR-RE.patchtext/x-diff; name=v5-0001-Prevent-internal-error-at-concurrent-CREATE-OR-RE.patchDownload
From 208f4bcc5b6f12ae95b2c2981786a441263b8a98 Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
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

#19Daniil Davydov
3danissimo@gmail.com
In reply to: Yugo Nagata (#18)
Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

Hi,

On Mon, Jun 30, 2025 at 3:47 PM Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Fri, 27 Jun 2025 18:53:02 +0700
Daniil Davydov <3danissimo@gmail.com> wrote:

This patch fixes postgres behavior if I first create a function and
then try to CREATE OR REPLACE it in concurrent transactions.
But if the function doesn't exist and I try to call CREATE OR REPLACE
in concurrent transactions, I will get an error.
I wrote about it in this thread [1] and Tom Lane said that this
behavior is kinda expected.
Just in case, I decided to mention it here anyway - perhaps you will
have other thoughts on this matter.

[1] /messages/by-id/CAJDiXghv2JF5zbLyyybokWKM+-GYsTG+hw7xseLNgJOJwf0+8w@mail.gmail.com

I agree with Tom Lane that the behavior is expected, although it would be better
if the error message were more user-friendly. We could improve it by checking the
unique constraint before calling index_insert in CatalogIndexInsert.

As far as I understand, unique constraint checking is specific for
each index access method.
Thus, to implement the proposed idea, you will have to create a
separate callback for check_unique function.
It doesn't seem like a very neat solution, but there aren't many other
options left.

I would suggest intercepting the error (via PG_CATCH), and if it has
the ERRCODE_UNIQUE_VIOLATION code, change the error message (more
precisely, throw another error with the desired message).
If we caught an error during the CatalogTupleInsert call, we can be
sure that the problem is in concurrent execution, because before the
insertion, we checked that such a tuple does not exist.

What do you think? And in general, are you going to fix this behavior
within this thread?

P.S.
Thank you for updating the patch.

--
Best regards,
Daniil Davydov

#20Yugo Nagata
nagata@sraoss.co.jp
In reply to: Daniil Davydov (#19)
4 attachment(s)
Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

On Mon, 30 Jun 2025 18:32:47 +0700
Daniil Davydov <3danissimo@gmail.com> wrote:

Hi,

On Mon, Jun 30, 2025 at 3:47 PM Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Fri, 27 Jun 2025 18:53:02 +0700
Daniil Davydov <3danissimo@gmail.com> wrote:

This patch fixes postgres behavior if I first create a function and
then try to CREATE OR REPLACE it in concurrent transactions.
But if the function doesn't exist and I try to call CREATE OR REPLACE
in concurrent transactions, I will get an error.
I wrote about it in this thread [1] and Tom Lane said that this
behavior is kinda expected.
Just in case, I decided to mention it here anyway - perhaps you will
have other thoughts on this matter.

[1] /messages/by-id/CAJDiXghv2JF5zbLyyybokWKM+-GYsTG+hw7xseLNgJOJwf0+8w@mail.gmail.com

I agree with Tom Lane that the behavior is expected, although it would be better
if the error message were more user-friendly. We could improve it by checking the
unique constraint before calling index_insert in CatalogIndexInsert.

As far as I understand, unique constraint checking is specific for
each index access method.
Thus, to implement the proposed idea, you will have to create a
separate callback for check_unique function.
It doesn't seem like a very neat solution, but there aren't many other
options left.

I believe check_exclusion_or_unique_constraint() can be used independently of
a specific index access method.

I would suggest intercepting the error (via PG_CATCH), and if it has
the ERRCODE_UNIQUE_VIOLATION code, change the error message (more
precisely, throw another error with the desired message).
If we caught an error during the CatalogTupleInsert call, we can be
sure that the problem is in concurrent execution, because before the
insertion, we checked that such a tuple does not exist.

What do you think? And in general, are you going to fix this behavior
within this thread?

Initially, I wasn't planning to do so, but I gave it a try and wrote a
patch to fix the issue based on my idea.

I've attached the patch as 0004. Other patches 0001-0003 are not changed.

Regards,
Yugo Nagata

--
Yugo Nagata <nagata@sraoss.co.jp>

Attachments:

v6-0004-Improve-error-reporting-for-concurrent-catalog-ob.patchtext/x-diff; name=v6-0004-Improve-error-reporting-for-concurrent-catalog-ob.patchDownload
From d74191ec28332152f1ecdc4d089a6d9744f9bf4f Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
Date: Tue, 1 Jul 2025 18:36:01 +0900
Subject: [PATCH v6 4/4] Improve error reporting for concurrent catalog object
 creation with same key

Previously, when multiple sessions attempted to create an object with the
same key, (e.g. CREATE TABLE with the same table name) concurrently, DDL
commands could fail with an error:

 ERROR:  duplicate key value violates unique constraint ....

This commit improves the behavior by reporting a more user-facing error
message in such cases, making it easier for users to understand the cause
of the failure.
---
 src/backend/catalog/indexing.c      | 22 +++++++++++++++++++++-
 src/backend/executor/execIndexing.c | 19 +++++++++++++++++++
 src/include/executor/executor.h     |  5 +++++
 3 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/src/backend/catalog/indexing.c b/src/backend/catalog/indexing.c
index 25c4b6bdc87..2919b27960c 100644
--- a/src/backend/catalog/indexing.c
+++ b/src/backend/catalog/indexing.c
@@ -91,7 +91,7 @@ CatalogIndexInsert(CatalogIndexState indstate, HeapTuple heapTuple,
 	 * table/index.
 	 */
 #ifndef USE_ASSERT_CHECKING
-	if (HeapTupleIsHeapOnly(heapTuple) && !onlySummarized)
+	if (HeapTupleIsHeapOenly(heapTuple) && !onlySummarized)
 		return;
 #endif
 
@@ -164,6 +164,26 @@ CatalogIndexInsert(CatalogIndexState indstate, HeapTuple heapTuple,
 					   values,
 					   isnull);
 
+		/* Check if a concurrent command inserted an entry with the same key */
+		if (index->rd_index->indisunique && IsCatalogRelation(heapRelation))
+		{
+			bool satisfied;
+			EState	*estate = CreateExecutorState();
+
+			BuildSpeculativeIndexInfo(index, indexInfo);
+			sartisfied = check_unique_constraint(heapRelation,
+												 index, indexInfo,
+												 &(heapTuple->t_self), values, isnull,
+												 estate);
+
+			if (!satisfied)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command inserted the key in unique index %s in a concurrent session.",
+								   RelationGetRelationName(index))));
+		}
+
 		/*
 		 * The index AM does the rest.
 		 */
diff --git a/src/backend/executor/execIndexing.c b/src/backend/executor/execIndexing.c
index bdf862b2406..889573feb6b 100644
--- a/src/backend/executor/execIndexing.c
+++ b/src/backend/executor/execIndexing.c
@@ -965,6 +965,25 @@ check_exclusion_constraint(Relation heap, Relation index,
 												CEOUC_WAIT, false, NULL);
 }
 
+/*
+ * Check for violation of a unique constraint
+ *
+ * This is a dumbed down version of check_exclusion_or_unique_constraint
+ * for external callers. They don't need all the special modes.
+ */
+bool
+check_unique_constraint(Relation heap, Relation index,
+						   IndexInfo *indexInfo,
+						   ItemPointer tupleid,
+						   const Datum *values, const bool *isnull,
+						   EState *estate)
+{
+	return check_exclusion_or_unique_constraint(heap, index, indexInfo, tupleid,
+												values, isnull,
+												estate, false,
+												CEOUC_WAIT, true, NULL);
+}
+
 /*
  * Check existing tuple's index values to see if it really matches the
  * exclusion condition against the new_values.  Returns true if conflict.
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index 104b059544d..2653e85e5ea 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -749,6 +749,11 @@ extern void check_exclusion_constraint(Relation heap, Relation index,
 									   ItemPointer tupleid,
 									   const Datum *values, const bool *isnull,
 									   EState *estate, bool newIndex);
+extern bool check_unique_constraint(Relation heap, Relation index,
+									IndexInfo *indexInfo,
+									ItemPointer tupleid,
+									const Datum *values, const bool *isnull,
+									EState *estate);
 
 /*
  * prototypes from functions in execReplication.c
-- 
2.43.0

v6-0003-Improve-error-reporting-for-concurrent-updates-on.patchtext/x-diff; name=v6-0003-Improve-error-reporting-for-concurrent-updates-on.patchDownload
From 7c9146f221a253aa3a150f6b6d5600ed0225e4e1 Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
Date: Thu, 5 Jun 2025 15:32:37 +0900
Subject: [PATCH v6 3/4] Improve error reporting for concurrent updates on
 system catalog tuples

Previously, when multiple sessions attempted to modify the same system
catalog tuple concurrently due to insufficient locking, DDL commands could
fail with an internal error:

ERROR:  tuple concurrently updated

This commit improves the behavior by reporting a more appropriate and
user-facing error message in such cases, making it easier for users to
understand the cause of the failure.
---
 src/backend/access/heap/heapam.c              | 34 ++++++++++++++++---
 .../expected/syscache-update-pruned.out       |  2 +-
 2 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 0dcd6ee817e..712b7ded6d2 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -3194,6 +3194,7 @@ simple_heap_delete(Relation relation, ItemPointer tid)
 {
 	TM_Result	result;
 	TM_FailureData tmfd;
+	bool is_catalog = IsCatalogRelation(relation);
 
 	result = heap_delete(relation, tid,
 						 GetCurrentCommandId(true), InvalidSnapshot,
@@ -3211,11 +3212,23 @@ simple_heap_delete(Relation relation, ItemPointer tid)
 			break;
 
 		case TM_Updated:
-			elog(ERROR, "tuple concurrently updated");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command modified the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently updated");
 			break;
 
 		case TM_Deleted:
-			elog(ERROR, "tuple concurrently deleted");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command dropped the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently deleted");
 			break;
 
 		default:
@@ -4486,6 +4499,7 @@ simple_heap_update(Relation relation, ItemPointer otid, HeapTuple tup,
 	TM_Result	result;
 	TM_FailureData tmfd;
 	LockTupleMode lockmode;
+	bool is_catalog = IsCatalogRelation(relation);
 
 	result = heap_update(relation, otid, tup,
 						 GetCurrentCommandId(true), InvalidSnapshot,
@@ -4503,11 +4517,23 @@ simple_heap_update(Relation relation, ItemPointer otid, HeapTuple tup,
 			break;
 
 		case TM_Updated:
-			elog(ERROR, "tuple concurrently updated");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command modified the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently updated");
 			break;
 
 		case TM_Deleted:
-			elog(ERROR, "tuple concurrently deleted");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command dropped the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently deleted");
 			break;
 
 		default:
diff --git a/src/test/modules/injection_points/expected/syscache-update-pruned.out b/src/test/modules/injection_points/expected/syscache-update-pruned.out
index a6a4e8db996..231545a6cbb 100644
--- a/src/test/modules/injection_points/expected/syscache-update-pruned.out
+++ b/src/test/modules/injection_points/expected/syscache-update-pruned.out
@@ -20,7 +20,7 @@ step wakegrant4:
 	SELECT FROM injection_points_wakeup('heap_update-before-pin');
  <waiting ...>
 step grant1: <... completed>
-ERROR:  tuple concurrently deleted
+ERROR:  operation failed due to a concurrent command
 step wakegrant4: <... completed>
 
 starting permutation: cachefill1 at2 waitprunable4 vac4 grant1 wakeinval4 mkrels4 wakegrant4
-- 
2.43.0

v6-0002-Prevent-internal-error-caused-by-concurrent-ALTER.patchtext/x-diff; name=v6-0002-Prevent-internal-error-caused-by-concurrent-ALTER.patchDownload
From 8e5aa491ee6754d55001f568557454fcfd65eb41 Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
Date: Mon, 31 Mar 2025 20:17:07 +0900
Subject: [PATCH v6 2/4] Prevent internal error caused by concurrent ALTER
 FUNCTION

Previously, concurrent ALTER 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, an appropriate error is raised to indicate the
object no longer exists.
---
 src/backend/commands/functioncmds.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index 0335e982b31..5bb03153c0d 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"
@@ -1383,9 +1384,21 @@ AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt)
 
 	ObjectAddressSet(address, ProcedureRelationId, 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)) /* should not happen */
-		elog(ERROR, "cache lookup failed for function %u", 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);
 
-- 
2.43.0

v6-0001-Prevent-internal-error-at-concurrent-CREATE-OR-RE.patchtext/x-diff; name=v6-0001-Prevent-internal-error-at-concurrent-CREATE-OR-RE.patchDownload
From 208f4bcc5b6f12ae95b2c2981786a441263b8a98 Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
Date: Mon, 31 Mar 2025 18:46:58 +0900
Subject: [PATCH v6 1/4] 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

#21Daniil Davydov
3danissimo@gmail.com
In reply to: Yugo Nagata (#20)
Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

Hi,

On Tue, Jul 1, 2025 at 5:47 PM Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Mon, 30 Jun 2025 18:32:47 +0700
Daniil Davydov <3danissimo@gmail.com> wrote:

On Mon, Jun 30, 2025 at 3:47 PM Yugo Nagata <nagata@sraoss.co.jp> wrote:

I agree with Tom Lane that the behavior is expected, although it would be better
if the error message were more user-friendly. We could improve it by checking the
unique constraint before calling index_insert in CatalogIndexInsert.

As far as I understand, unique constraint checking is specific for
each index access method.
Thus, to implement the proposed idea, you will have to create a
separate callback for check_unique function.
It doesn't seem like a very neat solution, but there aren't many other
options left.

I believe check_exclusion_or_unique_constraint() can be used independently of
a specific index access method.

I would suggest intercepting the error (via PG_CATCH), and if it has
the ERRCODE_UNIQUE_VIOLATION code, change the error message (more
precisely, throw another error with the desired message).
If we caught an error during the CatalogTupleInsert call, we can be
sure that the problem is in concurrent execution, because before the
insertion, we checked that such a tuple does not exist.

What do you think? And in general, are you going to fix this behavior
within this thread?

Initially, I wasn't planning to do so, but I gave it a try and wrote a
patch to fix the issue based on my idea.

Thanks for the patch! Some comments on it :
1)
I found two typos :
+    if (HeapTupleIsHeapOenly(heapTuple) && !onlySummarized)
and
+            sartisfied = check_unique_constraint(heapRelation,

2)
CatalogIndexInsert is kinda "popular" function. It can be called in
different situations, not in all of which a violation of unique
constraint means an error due to competitiveness.

For example, with this patch such a query : "CREATE TYPE mood AS ENUM
('happy', 'sad', 'happy');"
Will throw this error : "operation failed due to a concurrent command"
Of course, it isn't true.

That is why I suggested handling unique violations exactly inside
ProcedureCreate - the only place where we can be sure about reasons of
error.

--
Best regards,
Daniil Davydov

#22Yugo Nagata
nagata@sraoss.co.jp
In reply to: Daniil Davydov (#21)
4 attachment(s)
Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

On Tue, 1 Jul 2025 18:56:11 +0700
Daniil Davydov <3danissimo@gmail.com> wrote:

Hi,

On Tue, Jul 1, 2025 at 5:47 PM Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Mon, 30 Jun 2025 18:32:47 +0700
Daniil Davydov <3danissimo@gmail.com> wrote:

On Mon, Jun 30, 2025 at 3:47 PM Yugo Nagata <nagata@sraoss.co.jp> wrote:

I agree with Tom Lane that the behavior is expected, although it would be better
if the error message were more user-friendly. We could improve it by checking the
unique constraint before calling index_insert in CatalogIndexInsert.

As far as I understand, unique constraint checking is specific for
each index access method.
Thus, to implement the proposed idea, you will have to create a
separate callback for check_unique function.
It doesn't seem like a very neat solution, but there aren't many other
options left.

I believe check_exclusion_or_unique_constraint() can be used independently of
a specific index access method.

I would suggest intercepting the error (via PG_CATCH), and if it has
the ERRCODE_UNIQUE_VIOLATION code, change the error message (more
precisely, throw another error with the desired message).
If we caught an error during the CatalogTupleInsert call, we can be
sure that the problem is in concurrent execution, because before the
insertion, we checked that such a tuple does not exist.

What do you think? And in general, are you going to fix this behavior
within this thread?

Initially, I wasn't planning to do so, but I gave it a try and wrote a
patch to fix the issue based on my idea.

Thanks for the patch! Some comments on it :
1)
I found two typos :
+    if (HeapTupleIsHeapOenly(heapTuple) && !onlySummarized)
and
+            sartisfied = check_unique_constraint(heapRelation,

Thank you for pointing out them. Fixed.

2)
CatalogIndexInsert is kinda "popular" function. It can be called in
different situations, not in all of which a violation of unique
constraint means an error due to competitiveness.

For example, with this patch such a query : "CREATE TYPE mood AS ENUM
('happy', 'sad', 'happy');"
Will throw this error : "operation failed due to a concurrent command"
Of course, it isn't true

You're right — this error is not caused by a concurrent command.
However, I believe the error message in cases like creating an ENUM type with
duplicate labels could be improved to explain the issue more clearly, rather
than just reporting it as a unique constraint violation.

In any case, a unique constraint violation in a system catalog is not necessarily
due to concurrent DDL. Therefore, the error message shouldn't suggest that as the
only cause. Instead, it should clearly report the constraint violation as the primary
issue, and mention concurrent DDL as just one possible explanation in HINT.

I've updated the patch accordingly to reflect this direction in the error message.

ERROR: operation failed due to duplicate key object
DETAIL: Key (proname, proargtypes, pronamespace)=(fnc, , 2200) already exists in unique index pg_proc_proname_args_nsp_index.
HINT: Another command might have created a object with the same key in a concurrent session.

However, as a result, the message ends up being similar to the current one raised
by the btree code, so the overall improvement in user-friendliness might be limited.

That is why I suggested handling unique violations exactly inside
ProcedureCreate - the only place where we can be sure about reasons of
error.

If we were to fix the error message outside of CatalogIndexInsert, we would need to
modify CatalogTupleInsert, CatalogTupleUpdate, and related functions to allow them to
report the failure appropriately.

You suggested using PG_TRY/PG_CATCH, but these do not suppress the error message from
the btree code, so this approach seems not to fully address the issue.

Moreover, the places affected are not limited to ProcedureCreate, for example,
concurrent CREATE TABLE commands can also lead to the same situation, and possibly
other commands as well. Therefore, I think it would be sufficient if the
improved message in CatalogIndexInsert makes sense on its own.

Regards,
Yugo Nagata

--
Yugo Nagata <nagata@sraoss.co.jp>

Attachments:

v7-0004-Improve-error-reporting-for-unique-key-violations.patchtext/x-diff; name=v7-0004-Improve-error-reporting-for-unique-key-violations.patchDownload
From 40fc14a0a15aa85bcaf863007e972c8fe6250fe2 Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
Date: Tue, 1 Jul 2025 18:36:01 +0900
Subject: [PATCH v7 4/4] Improve error reporting for unique key violations in
 system catalogs

Previously, when a unique constraint violation occurred in a system catalog,
typically due to a concurrent session creating an object with the same key,
a low-level error like the following was raised by nbtree code:

 ERROR:  duplicate key value violates unique constraint ...

However, this message is not very user-friendly, as users are not directly
inserting rows into the system catalogs.

This commit improves the error reporting by generating a more descriptive and
user-facing error message in such cases, making it easier to understand the
cause of the failure and its likely relation to concurrent DDL activity.
---
 src/backend/catalog/indexing.c      | 24 ++++++++++++++++++++++++
 src/backend/executor/execIndexing.c | 19 +++++++++++++++++++
 src/include/executor/executor.h     |  5 +++++
 3 files changed, 48 insertions(+)

diff --git a/src/backend/catalog/indexing.c b/src/backend/catalog/indexing.c
index 25c4b6bdc87..5d91eeae16f 100644
--- a/src/backend/catalog/indexing.c
+++ b/src/backend/catalog/indexing.c
@@ -164,6 +164,30 @@ CatalogIndexInsert(CatalogIndexState indstate, HeapTuple heapTuple,
 					   values,
 					   isnull);
 
+		/* Check if a concurrent command inserted an entry with the same key */
+		if (index->rd_index->indisunique && IsCatalogRelation(heapRelation))
+		{
+			bool	satisfied;
+			EState  *estate = CreateExecutorState();
+
+			BuildSpeculativeIndexInfo(index, indexInfo);
+			satisfied = check_unique_constraint(heapRelation,
+												 index, indexInfo,
+												 &(heapTuple->t_self), values, isnull,
+												 estate);
+
+			if (!satisfied)
+			{
+				char *key_desc = BuildIndexValueDescription(index, values, isnull);
+				ereport(ERROR,
+						(errcode(ERRCODE_UNIQUE_VIOLATION),
+						 errmsg("could not create object because a conflicting object already exists"),
+						 errdetail("Key %s conflicts with existing entry in unique index %s.",
+								   key_desc, RelationGetRelationName(index)),
+						 errhint("Another session might have created an object with the same key concurrently.")));
+			}
+		}
+
 		/*
 		 * The index AM does the rest.
 		 */
diff --git a/src/backend/executor/execIndexing.c b/src/backend/executor/execIndexing.c
index bdf862b2406..889573feb6b 100644
--- a/src/backend/executor/execIndexing.c
+++ b/src/backend/executor/execIndexing.c
@@ -965,6 +965,25 @@ check_exclusion_constraint(Relation heap, Relation index,
 												CEOUC_WAIT, false, NULL);
 }
 
+/*
+ * Check for violation of a unique constraint
+ *
+ * This is a dumbed down version of check_exclusion_or_unique_constraint
+ * for external callers. They don't need all the special modes.
+ */
+bool
+check_unique_constraint(Relation heap, Relation index,
+						   IndexInfo *indexInfo,
+						   ItemPointer tupleid,
+						   const Datum *values, const bool *isnull,
+						   EState *estate)
+{
+	return check_exclusion_or_unique_constraint(heap, index, indexInfo, tupleid,
+												values, isnull,
+												estate, false,
+												CEOUC_WAIT, true, NULL);
+}
+
 /*
  * Check existing tuple's index values to see if it really matches the
  * exclusion condition against the new_values.  Returns true if conflict.
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index 104b059544d..2653e85e5ea 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -749,6 +749,11 @@ extern void check_exclusion_constraint(Relation heap, Relation index,
 									   ItemPointer tupleid,
 									   const Datum *values, const bool *isnull,
 									   EState *estate, bool newIndex);
+extern bool check_unique_constraint(Relation heap, Relation index,
+									IndexInfo *indexInfo,
+									ItemPointer tupleid,
+									const Datum *values, const bool *isnull,
+									EState *estate);
 
 /*
  * prototypes from functions in execReplication.c
-- 
2.43.0

v7-0003-Improve-error-reporting-for-concurrent-updates-on.patchtext/x-diff; name=v7-0003-Improve-error-reporting-for-concurrent-updates-on.patchDownload
From 7c9146f221a253aa3a150f6b6d5600ed0225e4e1 Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
Date: Thu, 5 Jun 2025 15:32:37 +0900
Subject: [PATCH v7 3/4] Improve error reporting for concurrent updates on
 system catalog tuples

Previously, when multiple sessions attempted to modify the same system
catalog tuple concurrently due to insufficient locking, DDL commands could
fail with an internal error:

ERROR:  tuple concurrently updated

This commit improves the behavior by reporting a more appropriate and
user-facing error message in such cases, making it easier for users to
understand the cause of the failure.
---
 src/backend/access/heap/heapam.c              | 34 ++++++++++++++++---
 .../expected/syscache-update-pruned.out       |  2 +-
 2 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 0dcd6ee817e..712b7ded6d2 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -3194,6 +3194,7 @@ simple_heap_delete(Relation relation, ItemPointer tid)
 {
 	TM_Result	result;
 	TM_FailureData tmfd;
+	bool is_catalog = IsCatalogRelation(relation);
 
 	result = heap_delete(relation, tid,
 						 GetCurrentCommandId(true), InvalidSnapshot,
@@ -3211,11 +3212,23 @@ simple_heap_delete(Relation relation, ItemPointer tid)
 			break;
 
 		case TM_Updated:
-			elog(ERROR, "tuple concurrently updated");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command modified the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently updated");
 			break;
 
 		case TM_Deleted:
-			elog(ERROR, "tuple concurrently deleted");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command dropped the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently deleted");
 			break;
 
 		default:
@@ -4486,6 +4499,7 @@ simple_heap_update(Relation relation, ItemPointer otid, HeapTuple tup,
 	TM_Result	result;
 	TM_FailureData tmfd;
 	LockTupleMode lockmode;
+	bool is_catalog = IsCatalogRelation(relation);
 
 	result = heap_update(relation, otid, tup,
 						 GetCurrentCommandId(true), InvalidSnapshot,
@@ -4503,11 +4517,23 @@ simple_heap_update(Relation relation, ItemPointer otid, HeapTuple tup,
 			break;
 
 		case TM_Updated:
-			elog(ERROR, "tuple concurrently updated");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command modified the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently updated");
 			break;
 
 		case TM_Deleted:
-			elog(ERROR, "tuple concurrently deleted");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command dropped the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently deleted");
 			break;
 
 		default:
diff --git a/src/test/modules/injection_points/expected/syscache-update-pruned.out b/src/test/modules/injection_points/expected/syscache-update-pruned.out
index a6a4e8db996..231545a6cbb 100644
--- a/src/test/modules/injection_points/expected/syscache-update-pruned.out
+++ b/src/test/modules/injection_points/expected/syscache-update-pruned.out
@@ -20,7 +20,7 @@ step wakegrant4:
 	SELECT FROM injection_points_wakeup('heap_update-before-pin');
  <waiting ...>
 step grant1: <... completed>
-ERROR:  tuple concurrently deleted
+ERROR:  operation failed due to a concurrent command
 step wakegrant4: <... completed>
 
 starting permutation: cachefill1 at2 waitprunable4 vac4 grant1 wakeinval4 mkrels4 wakegrant4
-- 
2.43.0

v7-0002-Prevent-internal-error-caused-by-concurrent-ALTER.patchtext/x-diff; name=v7-0002-Prevent-internal-error-caused-by-concurrent-ALTER.patchDownload
From 8e5aa491ee6754d55001f568557454fcfd65eb41 Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
Date: Mon, 31 Mar 2025 20:17:07 +0900
Subject: [PATCH v7 2/4] Prevent internal error caused by concurrent ALTER
 FUNCTION

Previously, concurrent ALTER 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, an appropriate error is raised to indicate the
object no longer exists.
---
 src/backend/commands/functioncmds.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index 0335e982b31..5bb03153c0d 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"
@@ -1383,9 +1384,21 @@ AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt)
 
 	ObjectAddressSet(address, ProcedureRelationId, 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)) /* should not happen */
-		elog(ERROR, "cache lookup failed for function %u", 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);
 
-- 
2.43.0

v7-0001-Prevent-internal-error-at-concurrent-CREATE-OR-RE.patchtext/x-diff; name=v7-0001-Prevent-internal-error-at-concurrent-CREATE-OR-RE.patchDownload
From 208f4bcc5b6f12ae95b2c2981786a441263b8a98 Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
Date: Mon, 31 Mar 2025 18:46:58 +0900
Subject: [PATCH v7 1/4] 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

#23Yugo Nagata
nagata@sraoss.co.jp
In reply to: Yugo Nagata (#22)
Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

On Thu, 3 Jul 2025 23:18:12 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Tue, 1 Jul 2025 18:56:11 +0700
Daniil Davydov <3danissimo@gmail.com> wrote:

For example, with this patch such a query : "CREATE TYPE mood AS ENUM
('happy', 'sad', 'happy');"
Will throw this error : "operation failed due to a concurrent command"
Of course, it isn't true

You're right — this error is not caused by a concurrent command.
However, I believe the error message in cases like creating an ENUM type with
duplicate labels could be improved to explain the issue more clearly, rather
than just reporting it as a unique constraint violation.

I have submitted a patch addressing this in a separate thread [1]/messages/by-id/20250704000402.37e605ab0c59c300965a17ee@sraoss.co.jp.

[1]: /messages/by-id/20250704000402.37e605ab0c59c300965a17ee@sraoss.co.jp

Regards,
Yugo Nagata

--
Yugo Nagata <nagata@sraoss.co.jp>

#24Daniil Davydov
3danissimo@gmail.com
In reply to: Yugo Nagata (#22)
Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

Hi,

On Thu, Jul 3, 2025 at 9:18 PM Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Tue, 1 Jul 2025 18:56:11 +0700
Daniil Davydov <3danissimo@gmail.com> wrote:

CatalogIndexInsert is kinda "popular" function. It can be called in
different situations, not in all of which a violation of unique
constraint means an error due to competitiveness.

For example, with this patch such a query : "CREATE TYPE mood AS ENUM
('happy', 'sad', 'happy');"
Will throw this error : "operation failed due to a concurrent command"
Of course, it isn't true

You're right — this error is not caused by a concurrent command.
However, I believe the error message in cases like creating an ENUM type with
duplicate labels could be improved to explain the issue more clearly, rather
than just reporting it as a unique constraint violation.

In any case, a unique constraint violation in a system catalog is not necessarily
due to concurrent DDL. Therefore, the error message shouldn't suggest that as the
only cause. Instead, it should clearly report the constraint violation as the primary
issue, and mention concurrent DDL as just one possible explanation in HINT.

I've updated the patch accordingly to reflect this direction in the error message.

ERROR: operation failed due to duplicate key object
DETAIL: Key (proname, proargtypes, pronamespace)=(fnc, , 2200) already exists in unique index pg_proc_proname_args_nsp_index.
HINT: Another command might have created a object with the same key in a concurrent session.

However, as a result, the message ends up being similar to the current one raised
by the btree code, so the overall improvement in user-friendliness might be limited.

Thanks for updating the patch!
+1 for adding such a hint for this error.

That is why I suggested handling unique violations exactly inside
ProcedureCreate - the only place where we can be sure about reasons of
error.

If we were to fix the error message outside of CatalogIndexInsert, we would need to
modify CatalogTupleInsert, CatalogTupleUpdate, and related functions to allow them to
report the failure appropriately.

You suggested using PG_TRY/PG_CATCH, but these do not suppress the error message from
the btree code, so this approach seems not to fully address the issue.

Moreover, the places affected are not limited to ProcedureCreate, for example,
concurrent CREATE TABLE commands can also lead to the same situation, and possibly
other commands as well.

Actually, we can suppress errors from btree (by flushing error context
and creating another), but it doesn't look like the best design
decision.
It was an idea for one concrete error fix. Anyway,I like the
correction you suggested better.

--
Best regards,
Daniil Davydov

#25Yugo Nagata
nagata@sraoss.co.jp
In reply to: Daniil Davydov (#24)
Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

On Fri, 4 Jul 2025 10:48:26 +0700
Daniil Davydov <3danissimo@gmail.com> wrote:

Hi,

On Thu, Jul 3, 2025 at 9:18 PM Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Tue, 1 Jul 2025 18:56:11 +0700
Daniil Davydov <3danissimo@gmail.com> wrote:

CatalogIndexInsert is kinda "popular" function. It can be called in
different situations, not in all of which a violation of unique
constraint means an error due to competitiveness.

For example, with this patch such a query : "CREATE TYPE mood AS ENUM
('happy', 'sad', 'happy');"
Will throw this error : "operation failed due to a concurrent command"
Of course, it isn't true

You're right — this error is not caused by a concurrent command.
However, I believe the error message in cases like creating an ENUM type with
duplicate labels could be improved to explain the issue more clearly, rather
than just reporting it as a unique constraint violation.

In any case, a unique constraint violation in a system catalog is not necessarily
due to concurrent DDL. Therefore, the error message shouldn't suggest that as the
only cause. Instead, it should clearly report the constraint violation as the primary
issue, and mention concurrent DDL as just one possible explanation in HINT.

I've updated the patch accordingly to reflect this direction in the error message.

ERROR: operation failed due to duplicate key object
DETAIL: Key (proname, proargtypes, pronamespace)=(fnc, , 2200) already exists in unique index pg_proc_proname_args_nsp_index.
HINT: Another command might have created a object with the same key in a concurrent session.

However, as a result, the message ends up being similar to the current one raised
by the btree code, so the overall improvement in user-friendliness might be limited.

Thanks for updating the patch!
+1 for adding such a hint for this error.

That is why I suggested handling unique violations exactly inside
ProcedureCreate - the only place where we can be sure about reasons of
error.

If we were to fix the error message outside of CatalogIndexInsert, we would need to
modify CatalogTupleInsert, CatalogTupleUpdate, and related functions to allow them to
report the failure appropriately.

You suggested using PG_TRY/PG_CATCH, but these do not suppress the error message from
the btree code, so this approach seems not to fully address the issue.

Moreover, the places affected are not limited to ProcedureCreate, for example,
concurrent CREATE TABLE commands can also lead to the same situation, and possibly
other commands as well.

Actually, we can suppress errors from btree (by flushing error context
and creating another),

Right. That was my misunderstanding.

Regards,
Yugo Nagata

--
Yugo Nagata <nagata@sraoss.co.jp>

#26Yugo Nagata
nagata@sraoss.co.jp
In reply to: Yugo Nagata (#25)
4 attachment(s)
Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

On Fri, 4 Jul 2025 14:58:05 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Fri, 4 Jul 2025 10:48:26 +0700
Daniil Davydov <3danissimo@gmail.com> wrote:

Hi,

On Thu, Jul 3, 2025 at 9:18 PM Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Tue, 1 Jul 2025 18:56:11 +0700
Daniil Davydov <3danissimo@gmail.com> wrote:

CatalogIndexInsert is kinda "popular" function. It can be called in
different situations, not in all of which a violation of unique
constraint means an error due to competitiveness.

For example, with this patch such a query : "CREATE TYPE mood AS ENUM
('happy', 'sad', 'happy');"
Will throw this error : "operation failed due to a concurrent command"
Of course, it isn't true

You're right — this error is not caused by a concurrent command.
However, I believe the error message in cases like creating an ENUM type with
duplicate labels could be improved to explain the issue more clearly, rather
than just reporting it as a unique constraint violation.

In any case, a unique constraint violation in a system catalog is not necessarily
due to concurrent DDL. Therefore, the error message shouldn't suggest that as the
only cause. Instead, it should clearly report the constraint violation as the primary
issue, and mention concurrent DDL as just one possible explanation in HINT.

I've updated the patch accordingly to reflect this direction in the error message.

ERROR: operation failed due to duplicate key object
DETAIL: Key (proname, proargtypes, pronamespace)=(fnc, , 2200) already exists in unique index pg_proc_proname_args_nsp_index.
HINT: Another command might have created a object with the same key in a concurrent session.

However, as a result, the message ends up being similar to the current one raised
by the btree code, so the overall improvement in user-friendliness might be limited.

Thanks for updating the patch!
+1 for adding such a hint for this error.

I've attached updated patches since I found some test failed.

Regards,
Yugo Nagata

--
Yugo Nagata <nagata@sraoss.co.jp>

Attachments:

v8-0004-Improve-error-reporting-for-unique-key-violations.patchtext/x-diff; name=v8-0004-Improve-error-reporting-for-unique-key-violations.patchDownload
From ea4e04b577a7f9b43044e2eda1f31a22344e82d2 Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
Date: Tue, 1 Jul 2025 18:36:01 +0900
Subject: [PATCH v8 4/4] Improve error reporting for unique key violations in
 system catalogs

Previously, when a unique constraint violation occurred in a system catalog,
typically due to a concurrent session creating an object with the same key,
a low-level error like the following was raised by nbtree code:

 ERROR:  duplicate key value violates unique constraint ...

However, this message is not very user-friendly, as users are not directly
inserting rows into the system catalogs.

This commit improves the error reporting by generating a more descriptive and
user-facing error message in such cases, making it easier to understand the
cause of the failure and its likely relation to concurrent DDL activity.
---
 contrib/test_decoding/expected/replorigin.out |  5 ++--
 src/backend/catalog/indexing.c                | 24 +++++++++++++++++++
 src/backend/executor/execIndexing.c           | 19 +++++++++++++++
 src/include/executor/executor.h               |  5 ++++
 .../expected/syscache-update-pruned.out       |  2 +-
 5 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/contrib/test_decoding/expected/replorigin.out b/contrib/test_decoding/expected/replorigin.out
index c85e1a01b23..9c30e9fb76d 100644
--- a/contrib/test_decoding/expected/replorigin.out
+++ b/contrib/test_decoding/expected/replorigin.out
@@ -39,8 +39,9 @@ SELECT pg_replication_origin_create('regress_test_decoding: regression_slot');
 
 -- ensure duplicate creations fail
 SELECT pg_replication_origin_create('regress_test_decoding: regression_slot');
-ERROR:  duplicate key value violates unique constraint "pg_replication_origin_roname_index"
-DETAIL:  Key (roname)=(regress_test_decoding: regression_slot) already exists.
+ERROR:  could not create object because a conflicting object already exists
+DETAIL:  Key (roname)=(regress_test_decoding: regression_slot) conflicts with existing entry in unique index pg_replication_origin_roname_index.
+HINT:  Another session might have created an object with the same key concurrently.
 --ensure deletions work (once)
 SELECT pg_replication_origin_create('regress_test_decoding: temp');
  pg_replication_origin_create 
diff --git a/src/backend/catalog/indexing.c b/src/backend/catalog/indexing.c
index 25c4b6bdc87..5d91eeae16f 100644
--- a/src/backend/catalog/indexing.c
+++ b/src/backend/catalog/indexing.c
@@ -164,6 +164,30 @@ CatalogIndexInsert(CatalogIndexState indstate, HeapTuple heapTuple,
 					   values,
 					   isnull);
 
+		/* Check if a concurrent command inserted an entry with the same key */
+		if (index->rd_index->indisunique && IsCatalogRelation(heapRelation))
+		{
+			bool	satisfied;
+			EState  *estate = CreateExecutorState();
+
+			BuildSpeculativeIndexInfo(index, indexInfo);
+			satisfied = check_unique_constraint(heapRelation,
+												 index, indexInfo,
+												 &(heapTuple->t_self), values, isnull,
+												 estate);
+
+			if (!satisfied)
+			{
+				char *key_desc = BuildIndexValueDescription(index, values, isnull);
+				ereport(ERROR,
+						(errcode(ERRCODE_UNIQUE_VIOLATION),
+						 errmsg("could not create object because a conflicting object already exists"),
+						 errdetail("Key %s conflicts with existing entry in unique index %s.",
+								   key_desc, RelationGetRelationName(index)),
+						 errhint("Another session might have created an object with the same key concurrently.")));
+			}
+		}
+
 		/*
 		 * The index AM does the rest.
 		 */
diff --git a/src/backend/executor/execIndexing.c b/src/backend/executor/execIndexing.c
index ca33a854278..f87ab861b44 100644
--- a/src/backend/executor/execIndexing.c
+++ b/src/backend/executor/execIndexing.c
@@ -965,6 +965,25 @@ check_exclusion_constraint(Relation heap, Relation index,
 												CEOUC_WAIT, false, NULL);
 }
 
+/*
+ * Check for violation of a unique constraint
+ *
+ * This is a dumbed down version of check_exclusion_or_unique_constraint
+ * for external callers. They don't need all the special modes.
+ */
+bool
+check_unique_constraint(Relation heap, Relation index,
+						   IndexInfo *indexInfo,
+						   ItemPointer tupleid,
+						   const Datum *values, const bool *isnull,
+						   EState *estate)
+{
+	return check_exclusion_or_unique_constraint(heap, index, indexInfo, tupleid,
+												values, isnull,
+												estate, false,
+												CEOUC_WAIT, true, NULL);
+}
+
 /*
  * Check existing tuple's index values to see if it really matches the
  * exclusion condition against the new_values.  Returns true if conflict.
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index 104b059544d..2653e85e5ea 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -749,6 +749,11 @@ extern void check_exclusion_constraint(Relation heap, Relation index,
 									   ItemPointer tupleid,
 									   const Datum *values, const bool *isnull,
 									   EState *estate, bool newIndex);
+extern bool check_unique_constraint(Relation heap, Relation index,
+									IndexInfo *indexInfo,
+									ItemPointer tupleid,
+									const Datum *values, const bool *isnull,
+									EState *estate);
 
 /*
  * prototypes from functions in execReplication.c
diff --git a/src/test/modules/injection_points/expected/syscache-update-pruned.out b/src/test/modules/injection_points/expected/syscache-update-pruned.out
index 231545a6cbb..79fd4bff43e 100644
--- a/src/test/modules/injection_points/expected/syscache-update-pruned.out
+++ b/src/test/modules/injection_points/expected/syscache-update-pruned.out
@@ -46,7 +46,7 @@ step wakegrant4:
 	SELECT FROM injection_points_wakeup('heap_update-before-pin');
  <waiting ...>
 step grant1: <... completed>
-ERROR:  duplicate key value violates unique constraint "pg_class_oid_index"
+ERROR:  could not create object because a conflicting object already exists
 step wakegrant4: <... completed>
 
 starting permutation: snap3 cachefill1 at2 mkrels4 r3 waitprunable4 vac4 grant1 wakeinval4 at4 wakegrant4 inspect4
-- 
2.43.0

v8-0003-Improve-error-reporting-for-concurrent-updates-on.patchtext/x-diff; name=v8-0003-Improve-error-reporting-for-concurrent-updates-on.patchDownload
From 2185f1859be11592f48a502b81fef008d13d9e00 Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
Date: Thu, 5 Jun 2025 15:32:37 +0900
Subject: [PATCH v8 3/4] Improve error reporting for concurrent updates on
 system catalog tuples

Previously, when multiple sessions attempted to modify the same system
catalog tuple concurrently due to insufficient locking, DDL commands could
fail with an internal error:

ERROR:  tuple concurrently updated

This commit improves the behavior by reporting a more appropriate and
user-facing error message in such cases, making it easier for users to
understand the cause of the failure.
---
 src/backend/access/heap/heapam.c              | 34 ++++++++++++++++---
 .../expected/syscache-update-pruned.out       |  2 +-
 .../expected/syscache-update-pruned_1.out     |  2 +-
 3 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 0dcd6ee817e..712b7ded6d2 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -3194,6 +3194,7 @@ simple_heap_delete(Relation relation, ItemPointer tid)
 {
 	TM_Result	result;
 	TM_FailureData tmfd;
+	bool is_catalog = IsCatalogRelation(relation);
 
 	result = heap_delete(relation, tid,
 						 GetCurrentCommandId(true), InvalidSnapshot,
@@ -3211,11 +3212,23 @@ simple_heap_delete(Relation relation, ItemPointer tid)
 			break;
 
 		case TM_Updated:
-			elog(ERROR, "tuple concurrently updated");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command modified the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently updated");
 			break;
 
 		case TM_Deleted:
-			elog(ERROR, "tuple concurrently deleted");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command dropped the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently deleted");
 			break;
 
 		default:
@@ -4486,6 +4499,7 @@ simple_heap_update(Relation relation, ItemPointer otid, HeapTuple tup,
 	TM_Result	result;
 	TM_FailureData tmfd;
 	LockTupleMode lockmode;
+	bool is_catalog = IsCatalogRelation(relation);
 
 	result = heap_update(relation, otid, tup,
 						 GetCurrentCommandId(true), InvalidSnapshot,
@@ -4503,11 +4517,23 @@ simple_heap_update(Relation relation, ItemPointer otid, HeapTuple tup,
 			break;
 
 		case TM_Updated:
-			elog(ERROR, "tuple concurrently updated");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command modified the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently updated");
 			break;
 
 		case TM_Deleted:
-			elog(ERROR, "tuple concurrently deleted");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command dropped the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently deleted");
 			break;
 
 		default:
diff --git a/src/test/modules/injection_points/expected/syscache-update-pruned.out b/src/test/modules/injection_points/expected/syscache-update-pruned.out
index a6a4e8db996..231545a6cbb 100644
--- a/src/test/modules/injection_points/expected/syscache-update-pruned.out
+++ b/src/test/modules/injection_points/expected/syscache-update-pruned.out
@@ -20,7 +20,7 @@ step wakegrant4:
 	SELECT FROM injection_points_wakeup('heap_update-before-pin');
  <waiting ...>
 step grant1: <... completed>
-ERROR:  tuple concurrently deleted
+ERROR:  operation failed due to a concurrent command
 step wakegrant4: <... completed>
 
 starting permutation: cachefill1 at2 waitprunable4 vac4 grant1 wakeinval4 mkrels4 wakegrant4
diff --git a/src/test/modules/injection_points/expected/syscache-update-pruned_1.out b/src/test/modules/injection_points/expected/syscache-update-pruned_1.out
index 4dca2b86bc8..e94a8b8cdeb 100644
--- a/src/test/modules/injection_points/expected/syscache-update-pruned_1.out
+++ b/src/test/modules/injection_points/expected/syscache-update-pruned_1.out
@@ -73,7 +73,7 @@ step wakegrant4:
 	SELECT FROM injection_points_wakeup('heap_update-before-pin');
  <waiting ...>
 step grant1: <... completed>
-ERROR:  tuple concurrently updated
+ERROR:  operation failed due to a concurrent command
 step wakegrant4: <... completed>
 step inspect4: 
 	SELECT relhastriggers, relhassubclass FROM pg_class
-- 
2.43.0

v8-0002-Prevent-internal-error-caused-by-concurrent-ALTER.patchtext/x-diff; name=v8-0002-Prevent-internal-error-caused-by-concurrent-ALTER.patchDownload
From d5ff4d5d13a0738cbba03ebe68b7dcacc6350478 Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
Date: Mon, 31 Mar 2025 20:17:07 +0900
Subject: [PATCH v8 2/4] Prevent internal error caused by concurrent ALTER
 FUNCTION

Previously, concurrent ALTER 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, an appropriate error is raised to indicate the
object no longer exists.
---
 src/backend/commands/functioncmds.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index 0335e982b31..5bb03153c0d 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"
@@ -1383,9 +1384,21 @@ AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt)
 
 	ObjectAddressSet(address, ProcedureRelationId, 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)) /* should not happen */
-		elog(ERROR, "cache lookup failed for function %u", 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);
 
-- 
2.43.0

v8-0001-Prevent-internal-error-at-concurrent-CREATE-OR-RE.patchtext/x-diff; name=v8-0001-Prevent-internal-error-at-concurrent-CREATE-OR-RE.patchDownload
From 976837201889ca1dcb81b2c0ad4506c9ab5b9018 Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
Date: Mon, 31 Mar 2025 18:46:58 +0900
Subject: [PATCH v8 1/4] 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

#27Yugo Nagata
nagata@sraoss.co.jp
In reply to: Yugo Nagata (#26)
4 attachment(s)
Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

On Thu, 17 Jul 2025 14:09:14 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Fri, 4 Jul 2025 14:58:05 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Fri, 4 Jul 2025 10:48:26 +0700
Daniil Davydov <3danissimo@gmail.com> wrote:

Hi,

On Thu, Jul 3, 2025 at 9:18 PM Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Tue, 1 Jul 2025 18:56:11 +0700
Daniil Davydov <3danissimo@gmail.com> wrote:

CatalogIndexInsert is kinda "popular" function. It can be called in
different situations, not in all of which a violation of unique
constraint means an error due to competitiveness.

For example, with this patch such a query : "CREATE TYPE mood AS ENUM
('happy', 'sad', 'happy');"
Will throw this error : "operation failed due to a concurrent command"
Of course, it isn't true

You're right — this error is not caused by a concurrent command.
However, I believe the error message in cases like creating an ENUM type with
duplicate labels could be improved to explain the issue more clearly, rather
than just reporting it as a unique constraint violation.

In any case, a unique constraint violation in a system catalog is not necessarily
due to concurrent DDL. Therefore, the error message shouldn't suggest that as the
only cause. Instead, it should clearly report the constraint violation as the primary
issue, and mention concurrent DDL as just one possible explanation in HINT.

I've updated the patch accordingly to reflect this direction in the error message.

ERROR: operation failed due to duplicate key object
DETAIL: Key (proname, proargtypes, pronamespace)=(fnc, , 2200) already exists in unique index pg_proc_proname_args_nsp_index.
HINT: Another command might have created a object with the same key in a concurrent session.

However, as a result, the message ends up being similar to the current one raised
by the btree code, so the overall improvement in user-friendliness might be limited.

Thanks for updating the patch!
+1 for adding such a hint for this error.

I've fixed the following cfbot failure:

SUMMARY: AddressSanitizer: heap-use-after-free /tmp/cirrus-ci-build/src/backend/catalog/pg_proc.c:406 in ProcedureCreate

This was caused by accessing oldproc->oid after oldtup had already been freed.

Regards,
Yugo Nagata

--
Yugo Nagata <nagata@sraoss.co.jp>

Attachments:

v9-0001-Prevent-internal-error-at-concurrent-CREATE-OR-RE.patchtext/x-diff; name=v9-0001-Prevent-internal-error-at-concurrent-CREATE-OR-RE.patchDownload
From d7685861562faf4c3f43f3189822a95e3e51803c Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
Date: Mon, 31 Mar 2025 18:46:58 +0900
Subject: [PATCH v9 1/4] 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 | 41 ++++++++++++++++++++++++++++-------
 1 file changed, 33 insertions(+), 8 deletions(-)

diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index 75b17fed15e..889e9b68a0d 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,48 @@ 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;
+		Oid			 procoid = oldproc->oid;
 
 		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, procoid, 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 +610,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

v9-0004-Improve-error-reporting-for-unique-key-violations.patchtext/x-diff; name=v9-0004-Improve-error-reporting-for-unique-key-violations.patchDownload
From ef2a98d8f07afb9d9ccdb11f218072b1c384599b Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
Date: Tue, 1 Jul 2025 18:36:01 +0900
Subject: [PATCH v9 4/4] Improve error reporting for unique key violations in
 system catalogs

Previously, when a unique constraint violation occurred in a system catalog,
typically due to a concurrent session creating an object with the same key,
a low-level error like the following was raised by nbtree code:

 ERROR:  duplicate key value violates unique constraint ...

However, this message is not very user-friendly, as users are not directly
inserting rows into the system catalogs.

This commit improves the error reporting by generating a more descriptive and
user-facing error message in such cases, making it easier to understand the
cause of the failure and its likely relation to concurrent DDL activity.
---
 contrib/test_decoding/expected/replorigin.out |  5 ++--
 src/backend/catalog/indexing.c                | 24 +++++++++++++++++++
 src/backend/executor/execIndexing.c           | 19 +++++++++++++++
 src/include/executor/executor.h               |  5 ++++
 .../expected/syscache-update-pruned.out       |  2 +-
 5 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/contrib/test_decoding/expected/replorigin.out b/contrib/test_decoding/expected/replorigin.out
index c85e1a01b23..9c30e9fb76d 100644
--- a/contrib/test_decoding/expected/replorigin.out
+++ b/contrib/test_decoding/expected/replorigin.out
@@ -39,8 +39,9 @@ SELECT pg_replication_origin_create('regress_test_decoding: regression_slot');
 
 -- ensure duplicate creations fail
 SELECT pg_replication_origin_create('regress_test_decoding: regression_slot');
-ERROR:  duplicate key value violates unique constraint "pg_replication_origin_roname_index"
-DETAIL:  Key (roname)=(regress_test_decoding: regression_slot) already exists.
+ERROR:  could not create object because a conflicting object already exists
+DETAIL:  Key (roname)=(regress_test_decoding: regression_slot) conflicts with existing entry in unique index pg_replication_origin_roname_index.
+HINT:  Another session might have created an object with the same key concurrently.
 --ensure deletions work (once)
 SELECT pg_replication_origin_create('regress_test_decoding: temp');
  pg_replication_origin_create 
diff --git a/src/backend/catalog/indexing.c b/src/backend/catalog/indexing.c
index 25c4b6bdc87..5d91eeae16f 100644
--- a/src/backend/catalog/indexing.c
+++ b/src/backend/catalog/indexing.c
@@ -164,6 +164,30 @@ CatalogIndexInsert(CatalogIndexState indstate, HeapTuple heapTuple,
 					   values,
 					   isnull);
 
+		/* Check if a concurrent command inserted an entry with the same key */
+		if (index->rd_index->indisunique && IsCatalogRelation(heapRelation))
+		{
+			bool	satisfied;
+			EState  *estate = CreateExecutorState();
+
+			BuildSpeculativeIndexInfo(index, indexInfo);
+			satisfied = check_unique_constraint(heapRelation,
+												 index, indexInfo,
+												 &(heapTuple->t_self), values, isnull,
+												 estate);
+
+			if (!satisfied)
+			{
+				char *key_desc = BuildIndexValueDescription(index, values, isnull);
+				ereport(ERROR,
+						(errcode(ERRCODE_UNIQUE_VIOLATION),
+						 errmsg("could not create object because a conflicting object already exists"),
+						 errdetail("Key %s conflicts with existing entry in unique index %s.",
+								   key_desc, RelationGetRelationName(index)),
+						 errhint("Another session might have created an object with the same key concurrently.")));
+			}
+		}
+
 		/*
 		 * The index AM does the rest.
 		 */
diff --git a/src/backend/executor/execIndexing.c b/src/backend/executor/execIndexing.c
index ca33a854278..f87ab861b44 100644
--- a/src/backend/executor/execIndexing.c
+++ b/src/backend/executor/execIndexing.c
@@ -965,6 +965,25 @@ check_exclusion_constraint(Relation heap, Relation index,
 												CEOUC_WAIT, false, NULL);
 }
 
+/*
+ * Check for violation of a unique constraint
+ *
+ * This is a dumbed down version of check_exclusion_or_unique_constraint
+ * for external callers. They don't need all the special modes.
+ */
+bool
+check_unique_constraint(Relation heap, Relation index,
+						   IndexInfo *indexInfo,
+						   ItemPointer tupleid,
+						   const Datum *values, const bool *isnull,
+						   EState *estate)
+{
+	return check_exclusion_or_unique_constraint(heap, index, indexInfo, tupleid,
+												values, isnull,
+												estate, false,
+												CEOUC_WAIT, true, NULL);
+}
+
 /*
  * Check existing tuple's index values to see if it really matches the
  * exclusion condition against the new_values.  Returns true if conflict.
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index 10dcea037c3..3190548c385 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -751,6 +751,11 @@ extern void check_exclusion_constraint(Relation heap, Relation index,
 									   ItemPointer tupleid,
 									   const Datum *values, const bool *isnull,
 									   EState *estate, bool newIndex);
+extern bool check_unique_constraint(Relation heap, Relation index,
+									IndexInfo *indexInfo,
+									ItemPointer tupleid,
+									const Datum *values, const bool *isnull,
+									EState *estate);
 
 /*
  * prototypes from functions in execReplication.c
diff --git a/src/test/modules/injection_points/expected/syscache-update-pruned.out b/src/test/modules/injection_points/expected/syscache-update-pruned.out
index 231545a6cbb..79fd4bff43e 100644
--- a/src/test/modules/injection_points/expected/syscache-update-pruned.out
+++ b/src/test/modules/injection_points/expected/syscache-update-pruned.out
@@ -46,7 +46,7 @@ step wakegrant4:
 	SELECT FROM injection_points_wakeup('heap_update-before-pin');
  <waiting ...>
 step grant1: <... completed>
-ERROR:  duplicate key value violates unique constraint "pg_class_oid_index"
+ERROR:  could not create object because a conflicting object already exists
 step wakegrant4: <... completed>
 
 starting permutation: snap3 cachefill1 at2 mkrels4 r3 waitprunable4 vac4 grant1 wakeinval4 at4 wakegrant4 inspect4
-- 
2.43.0

v9-0003-Improve-error-reporting-for-concurrent-updates-on.patchtext/x-diff; name=v9-0003-Improve-error-reporting-for-concurrent-updates-on.patchDownload
From e6c7b08014383a78dbffea027b3e482095cc661d Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
Date: Thu, 5 Jun 2025 15:32:37 +0900
Subject: [PATCH v9 3/4] Improve error reporting for concurrent updates on
 system catalog tuples

Previously, when multiple sessions attempted to modify the same system
catalog tuple concurrently due to insufficient locking, DDL commands could
fail with an internal error:

ERROR:  tuple concurrently updated

This commit improves the behavior by reporting a more appropriate and
user-facing error message in such cases, making it easier for users to
understand the cause of the failure.
---
 src/backend/access/heap/heapam.c              | 34 ++++++++++++++++---
 .../expected/syscache-update-pruned.out       |  2 +-
 .../expected/syscache-update-pruned_1.out     |  2 +-
 3 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 0dcd6ee817e..712b7ded6d2 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -3194,6 +3194,7 @@ simple_heap_delete(Relation relation, ItemPointer tid)
 {
 	TM_Result	result;
 	TM_FailureData tmfd;
+	bool is_catalog = IsCatalogRelation(relation);
 
 	result = heap_delete(relation, tid,
 						 GetCurrentCommandId(true), InvalidSnapshot,
@@ -3211,11 +3212,23 @@ simple_heap_delete(Relation relation, ItemPointer tid)
 			break;
 
 		case TM_Updated:
-			elog(ERROR, "tuple concurrently updated");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command modified the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently updated");
 			break;
 
 		case TM_Deleted:
-			elog(ERROR, "tuple concurrently deleted");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command dropped the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently deleted");
 			break;
 
 		default:
@@ -4486,6 +4499,7 @@ simple_heap_update(Relation relation, ItemPointer otid, HeapTuple tup,
 	TM_Result	result;
 	TM_FailureData tmfd;
 	LockTupleMode lockmode;
+	bool is_catalog = IsCatalogRelation(relation);
 
 	result = heap_update(relation, otid, tup,
 						 GetCurrentCommandId(true), InvalidSnapshot,
@@ -4503,11 +4517,23 @@ simple_heap_update(Relation relation, ItemPointer otid, HeapTuple tup,
 			break;
 
 		case TM_Updated:
-			elog(ERROR, "tuple concurrently updated");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command modified the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently updated");
 			break;
 
 		case TM_Deleted:
-			elog(ERROR, "tuple concurrently deleted");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command dropped the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently deleted");
 			break;
 
 		default:
diff --git a/src/test/modules/injection_points/expected/syscache-update-pruned.out b/src/test/modules/injection_points/expected/syscache-update-pruned.out
index a6a4e8db996..231545a6cbb 100644
--- a/src/test/modules/injection_points/expected/syscache-update-pruned.out
+++ b/src/test/modules/injection_points/expected/syscache-update-pruned.out
@@ -20,7 +20,7 @@ step wakegrant4:
 	SELECT FROM injection_points_wakeup('heap_update-before-pin');
  <waiting ...>
 step grant1: <... completed>
-ERROR:  tuple concurrently deleted
+ERROR:  operation failed due to a concurrent command
 step wakegrant4: <... completed>
 
 starting permutation: cachefill1 at2 waitprunable4 vac4 grant1 wakeinval4 mkrels4 wakegrant4
diff --git a/src/test/modules/injection_points/expected/syscache-update-pruned_1.out b/src/test/modules/injection_points/expected/syscache-update-pruned_1.out
index 4dca2b86bc8..e94a8b8cdeb 100644
--- a/src/test/modules/injection_points/expected/syscache-update-pruned_1.out
+++ b/src/test/modules/injection_points/expected/syscache-update-pruned_1.out
@@ -73,7 +73,7 @@ step wakegrant4:
 	SELECT FROM injection_points_wakeup('heap_update-before-pin');
  <waiting ...>
 step grant1: <... completed>
-ERROR:  tuple concurrently updated
+ERROR:  operation failed due to a concurrent command
 step wakegrant4: <... completed>
 step inspect4: 
 	SELECT relhastriggers, relhassubclass FROM pg_class
-- 
2.43.0

v9-0002-Prevent-internal-error-caused-by-concurrent-ALTER.patchtext/x-diff; name=v9-0002-Prevent-internal-error-caused-by-concurrent-ALTER.patchDownload
From 68970346e855a8dfb840ff808acb55795bdd6536 Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
Date: Mon, 31 Mar 2025 20:17:07 +0900
Subject: [PATCH v9 2/4] Prevent internal error caused by concurrent ALTER
 FUNCTION

Previously, concurrent ALTER 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, an appropriate error is raised to indicate the
object no longer exists.
---
 src/backend/commands/functioncmds.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index 0335e982b31..5bb03153c0d 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"
@@ -1383,9 +1384,21 @@ AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt)
 
 	ObjectAddressSet(address, ProcedureRelationId, 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)) /* should not happen */
-		elog(ERROR, "cache lookup failed for function %u", 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);
 
-- 
2.43.0

#28Yugo Nagata
nagata@sraoss.co.jp
In reply to: Yugo Nagata (#27)
4 attachment(s)
Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

On Wed, 20 Aug 2025 17:01:56 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Thu, 17 Jul 2025 14:09:14 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Fri, 4 Jul 2025 14:58:05 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Fri, 4 Jul 2025 10:48:26 +0700
Daniil Davydov <3danissimo@gmail.com> wrote:

Hi,

On Thu, Jul 3, 2025 at 9:18 PM Yugo Nagata <nagata@sraoss.co.jp> wrote:

On Tue, 1 Jul 2025 18:56:11 +0700
Daniil Davydov <3danissimo@gmail.com> wrote:

CatalogIndexInsert is kinda "popular" function. It can be called in
different situations, not in all of which a violation of unique
constraint means an error due to competitiveness.

For example, with this patch such a query : "CREATE TYPE mood AS ENUM
('happy', 'sad', 'happy');"
Will throw this error : "operation failed due to a concurrent command"
Of course, it isn't true

You're right — this error is not caused by a concurrent command.
However, I believe the error message in cases like creating an ENUM type with
duplicate labels could be improved to explain the issue more clearly, rather
than just reporting it as a unique constraint violation.

In any case, a unique constraint violation in a system catalog is not necessarily
due to concurrent DDL. Therefore, the error message shouldn't suggest that as the
only cause. Instead, it should clearly report the constraint violation as the primary
issue, and mention concurrent DDL as just one possible explanation in HINT.

I've updated the patch accordingly to reflect this direction in the error message.

ERROR: operation failed due to duplicate key object
DETAIL: Key (proname, proargtypes, pronamespace)=(fnc, , 2200) already exists in unique index pg_proc_proname_args_nsp_index.
HINT: Another command might have created a object with the same key in a concurrent session.

However, as a result, the message ends up being similar to the current one raised
by the btree code, so the overall improvement in user-friendliness might be limited.

Thanks for updating the patch!
+1 for adding such a hint for this error.

I've attached rebase patches.

Regards,
Yugo Nagata

--
Yugo Nagata <nagata@sraoss.co.jp>

Attachments:

v10-0004-Improve-error-reporting-for-unique-key-violation.patchtext/x-diff; name=v10-0004-Improve-error-reporting-for-unique-key-violation.patchDownload
From efb50c882964c7962483e55563a666cffafdebe6 Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
Date: Tue, 1 Jul 2025 18:36:01 +0900
Subject: [PATCH v10 4/4] Improve error reporting for unique key violations in
 system catalogs

Previously, when a unique constraint violation occurred in a system catalog,
typically due to a concurrent session creating an object with the same key,
a low-level error like the following was raised by nbtree code:

 ERROR:  duplicate key value violates unique constraint ...

However, this message is not very user-friendly, as users are not directly
inserting rows into the system catalogs.

This commit improves the error reporting by generating a more descriptive and
user-facing error message in such cases, making it easier to understand the
cause of the failure and its likely relation to concurrent DDL activity.
---
 contrib/test_decoding/expected/replorigin.out |  5 ++--
 src/backend/catalog/indexing.c                | 24 +++++++++++++++++++
 src/backend/executor/execIndexing.c           | 19 +++++++++++++++
 src/include/executor/executor.h               |  5 ++++
 .../expected/syscache-update-pruned.out       |  2 +-
 5 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/contrib/test_decoding/expected/replorigin.out b/contrib/test_decoding/expected/replorigin.out
index 29a9630c900..e46c9f17d4a 100644
--- a/contrib/test_decoding/expected/replorigin.out
+++ b/contrib/test_decoding/expected/replorigin.out
@@ -39,8 +39,9 @@ SELECT pg_replication_origin_create('regress_test_decoding: regression_slot');
 
 -- ensure duplicate creations fail
 SELECT pg_replication_origin_create('regress_test_decoding: regression_slot');
-ERROR:  duplicate key value violates unique constraint "pg_replication_origin_roname_index"
-DETAIL:  Key (roname)=(regress_test_decoding: regression_slot) already exists.
+ERROR:  could not create object because a conflicting object already exists
+DETAIL:  Key (roname)=(regress_test_decoding: regression_slot) conflicts with existing entry in unique index pg_replication_origin_roname_index.
+HINT:  Another session might have created an object with the same key concurrently.
 -- ensure inactive origin cannot be set as session one if pid is specified
 SELECT pg_replication_origin_session_setup('regress_test_decoding: regression_slot', -1);
 ERROR:  cannot use PID -1 for inactive replication origin with ID 1
diff --git a/src/backend/catalog/indexing.c b/src/backend/catalog/indexing.c
index 25c4b6bdc87..5d91eeae16f 100644
--- a/src/backend/catalog/indexing.c
+++ b/src/backend/catalog/indexing.c
@@ -164,6 +164,30 @@ CatalogIndexInsert(CatalogIndexState indstate, HeapTuple heapTuple,
 					   values,
 					   isnull);
 
+		/* Check if a concurrent command inserted an entry with the same key */
+		if (index->rd_index->indisunique && IsCatalogRelation(heapRelation))
+		{
+			bool	satisfied;
+			EState  *estate = CreateExecutorState();
+
+			BuildSpeculativeIndexInfo(index, indexInfo);
+			satisfied = check_unique_constraint(heapRelation,
+												 index, indexInfo,
+												 &(heapTuple->t_self), values, isnull,
+												 estate);
+
+			if (!satisfied)
+			{
+				char *key_desc = BuildIndexValueDescription(index, values, isnull);
+				ereport(ERROR,
+						(errcode(ERRCODE_UNIQUE_VIOLATION),
+						 errmsg("could not create object because a conflicting object already exists"),
+						 errdetail("Key %s conflicts with existing entry in unique index %s.",
+								   key_desc, RelationGetRelationName(index)),
+						 errhint("Another session might have created an object with the same key concurrently.")));
+			}
+		}
+
 		/*
 		 * The index AM does the rest.
 		 */
diff --git a/src/backend/executor/execIndexing.c b/src/backend/executor/execIndexing.c
index ca33a854278..f87ab861b44 100644
--- a/src/backend/executor/execIndexing.c
+++ b/src/backend/executor/execIndexing.c
@@ -965,6 +965,25 @@ check_exclusion_constraint(Relation heap, Relation index,
 												CEOUC_WAIT, false, NULL);
 }
 
+/*
+ * Check for violation of a unique constraint
+ *
+ * This is a dumbed down version of check_exclusion_or_unique_constraint
+ * for external callers. They don't need all the special modes.
+ */
+bool
+check_unique_constraint(Relation heap, Relation index,
+						   IndexInfo *indexInfo,
+						   ItemPointer tupleid,
+						   const Datum *values, const bool *isnull,
+						   EState *estate)
+{
+	return check_exclusion_or_unique_constraint(heap, index, indexInfo, tupleid,
+												values, isnull,
+												estate, false,
+												CEOUC_WAIT, true, NULL);
+}
+
 /*
  * Check existing tuple's index values to see if it really matches the
  * exclusion condition against the new_values.  Returns true if conflict.
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index 3248e78cd28..3d1decb54bf 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -752,6 +752,11 @@ extern void check_exclusion_constraint(Relation heap, Relation index,
 									   ItemPointer tupleid,
 									   const Datum *values, const bool *isnull,
 									   EState *estate, bool newIndex);
+extern bool check_unique_constraint(Relation heap, Relation index,
+									IndexInfo *indexInfo,
+									ItemPointer tupleid,
+									const Datum *values, const bool *isnull,
+									EState *estate);
 
 /*
  * prototypes from functions in execReplication.c
diff --git a/src/test/modules/injection_points/expected/syscache-update-pruned.out b/src/test/modules/injection_points/expected/syscache-update-pruned.out
index 231545a6cbb..79fd4bff43e 100644
--- a/src/test/modules/injection_points/expected/syscache-update-pruned.out
+++ b/src/test/modules/injection_points/expected/syscache-update-pruned.out
@@ -46,7 +46,7 @@ step wakegrant4:
 	SELECT FROM injection_points_wakeup('heap_update-before-pin');
  <waiting ...>
 step grant1: <... completed>
-ERROR:  duplicate key value violates unique constraint "pg_class_oid_index"
+ERROR:  could not create object because a conflicting object already exists
 step wakegrant4: <... completed>
 
 starting permutation: snap3 cachefill1 at2 mkrels4 r3 waitprunable4 vac4 grant1 wakeinval4 at4 wakegrant4 inspect4
-- 
2.43.0

v10-0003-Improve-error-reporting-for-concurrent-updates-o.patchtext/x-diff; name=v10-0003-Improve-error-reporting-for-concurrent-updates-o.patchDownload
From 99fc9302e85790b41bce49f9a1e6d4adf81fe886 Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
Date: Thu, 5 Jun 2025 15:32:37 +0900
Subject: [PATCH v10 3/4] Improve error reporting for concurrent updates on
 system catalog tuples

Previously, when multiple sessions attempted to modify the same system
catalog tuple concurrently due to insufficient locking, DDL commands could
fail with an internal error:

ERROR:  tuple concurrently updated

This commit improves the behavior by reporting a more appropriate and
user-facing error message in such cases, making it easier for users to
understand the cause of the failure.
---
 src/backend/access/heap/heapam.c              | 34 ++++++++++++++++---
 .../expected/syscache-update-pruned.out       |  2 +-
 .../expected/syscache-update-pruned_1.out     |  2 +-
 3 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index ed0c0c2dc9f..7e300559a7d 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -3202,6 +3202,7 @@ simple_heap_delete(Relation relation, ItemPointer tid)
 {
 	TM_Result	result;
 	TM_FailureData tmfd;
+	bool is_catalog = IsCatalogRelation(relation);
 
 	result = heap_delete(relation, tid,
 						 GetCurrentCommandId(true), InvalidSnapshot,
@@ -3219,11 +3220,23 @@ simple_heap_delete(Relation relation, ItemPointer tid)
 			break;
 
 		case TM_Updated:
-			elog(ERROR, "tuple concurrently updated");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command modified the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently updated");
 			break;
 
 		case TM_Deleted:
-			elog(ERROR, "tuple concurrently deleted");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command dropped the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently deleted");
 			break;
 
 		default:
@@ -4494,6 +4507,7 @@ simple_heap_update(Relation relation, ItemPointer otid, HeapTuple tup,
 	TM_Result	result;
 	TM_FailureData tmfd;
 	LockTupleMode lockmode;
+	bool is_catalog = IsCatalogRelation(relation);
 
 	result = heap_update(relation, otid, tup,
 						 GetCurrentCommandId(true), InvalidSnapshot,
@@ -4511,11 +4525,23 @@ simple_heap_update(Relation relation, ItemPointer otid, HeapTuple tup,
 			break;
 
 		case TM_Updated:
-			elog(ERROR, "tuple concurrently updated");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command modified the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently updated");
 			break;
 
 		case TM_Deleted:
-			elog(ERROR, "tuple concurrently deleted");
+			if (is_catalog)
+				ereport(ERROR,
+						(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+						 errmsg("operation failed due to a concurrent command"),
+						 errdetail("Another command dropped the same object in a concurrent session.")));
+			else
+				elog(ERROR, "tuple concurrently deleted");
 			break;
 
 		default:
diff --git a/src/test/modules/injection_points/expected/syscache-update-pruned.out b/src/test/modules/injection_points/expected/syscache-update-pruned.out
index a6a4e8db996..231545a6cbb 100644
--- a/src/test/modules/injection_points/expected/syscache-update-pruned.out
+++ b/src/test/modules/injection_points/expected/syscache-update-pruned.out
@@ -20,7 +20,7 @@ step wakegrant4:
 	SELECT FROM injection_points_wakeup('heap_update-before-pin');
  <waiting ...>
 step grant1: <... completed>
-ERROR:  tuple concurrently deleted
+ERROR:  operation failed due to a concurrent command
 step wakegrant4: <... completed>
 
 starting permutation: cachefill1 at2 waitprunable4 vac4 grant1 wakeinval4 mkrels4 wakegrant4
diff --git a/src/test/modules/injection_points/expected/syscache-update-pruned_1.out b/src/test/modules/injection_points/expected/syscache-update-pruned_1.out
index 4dca2b86bc8..e94a8b8cdeb 100644
--- a/src/test/modules/injection_points/expected/syscache-update-pruned_1.out
+++ b/src/test/modules/injection_points/expected/syscache-update-pruned_1.out
@@ -73,7 +73,7 @@ step wakegrant4:
 	SELECT FROM injection_points_wakeup('heap_update-before-pin');
  <waiting ...>
 step grant1: <... completed>
-ERROR:  tuple concurrently updated
+ERROR:  operation failed due to a concurrent command
 step wakegrant4: <... completed>
 step inspect4: 
 	SELECT relhastriggers, relhassubclass FROM pg_class
-- 
2.43.0

v10-0002-Prevent-internal-error-caused-by-concurrent-ALTE.patchtext/x-diff; name=v10-0002-Prevent-internal-error-caused-by-concurrent-ALTE.patchDownload
From e739cc916fc0d224d865531f12e7b476be5fa697 Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
Date: Mon, 31 Mar 2025 20:17:07 +0900
Subject: [PATCH v10 2/4] Prevent internal error caused by concurrent ALTER
 FUNCTION

Previously, concurrent ALTER 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, an appropriate error is raised to indicate the
object no longer exists.
---
 src/backend/commands/functioncmds.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index 0335e982b31..5bb03153c0d 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"
@@ -1383,9 +1384,21 @@ AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt)
 
 	ObjectAddressSet(address, ProcedureRelationId, 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)) /* should not happen */
-		elog(ERROR, "cache lookup failed for function %u", 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);
 
-- 
2.43.0

v10-0001-Prevent-internal-error-at-concurrent-CREATE-OR-R.patchtext/x-diff; name=v10-0001-Prevent-internal-error-at-concurrent-CREATE-OR-R.patchDownload
From a1b049c1c29e5b45d1371fb72106b82d816ae126 Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nagata@sraoss.co.jp>
Date: Mon, 31 Mar 2025 18:46:58 +0900
Subject: [PATCH v10 1/4] 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 | 41 ++++++++++++++++++++++++++++-------
 1 file changed, 33 insertions(+), 8 deletions(-)

diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index b89b9ccda0e..b459badf074 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,48 @@ 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;
+		Oid			 procoid = oldproc->oid;
 
 		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, procoid, 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 +610,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

#29Daniil Davydov
3danissimo@gmail.com
In reply to: Yugo Nagata (#28)
Re: Prevent internal error at concurrent CREATE OR REPLACE FUNCTION

Hi,

On Tue, Sep 30, 2025 at 9:02 AM Yugo Nagata <nagata@sraoss.co.jp> wrote:

I've attached rebase patches.

It seems redundant to me that in CatalogIndexInsert we first check the
unique constraint, and then pass the UNIQUE_CHECK_YES parameter
to the index_insert function call below. As far as I understand, after
check_unique_constraint we can be sure that everything is okay with
inserted values. Am I missing something?

Also, why should we add "IsCatalogRelation(heapRelation)" check inside
the CatalogIndexInsert function? We know for sure that a given table is a
catalog relation.

BTW, what do you think about adding an isolation test for a concurrent
"CREATE OR REPLACE FUNCTION" command? This is one of the
'problems' we're struggling with, but I don't see this case being explicitly
tested anywhere.

All other changes look good to me.

--
Best regards,
Daniil Davydov