From e1a4f7ab757cf841de795143a578da32ed3f030f Mon Sep 17 00:00:00 2001
From: Vignesh C <c_vigneshwaran@apple.com>
Date: Wed, 9 Sep 2026 09:40:32 +0530
Subject: [PATCH v3 1/2] AlterPublication(): revalidate publication after
 acquiring lock

AlterPublicationOptions() can use stale publication state when the
publication is modified concurrently while waiting for the publication
lock. In particular, a concurrent ALTER PUBLICATION ... SET ALL TABLES
can change puballtables, causing the validation to make decisions based
on the old value.

Similarly, a concurrent ALTER PUBLICATION can modify pg_publication_rel
while the command waits for the lock. The publication tuple is
re-fetched after acquiring the lock, but the relation validation is not
re-run, potentially leaving pg_publication_rel with a mixture of
inclusion and exclusion rows.

Acquire the publication lock before re-reading the publication state and
re-run CheckAlterPublication() after acquiring the lock so that both
publication options and relations are validated against the current
state.
---
 src/backend/commands/publicationcmds.c | 73 ++++++++++++++------------
 1 file changed, 39 insertions(+), 34 deletions(-)

diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c
index 96838730fe1..301050104dd 100644
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -1057,15 +1057,6 @@ AlterPublicationOptions(ParseState *pstate, AlterPublicationStmt *stmt,
 	if (!pubform->puballtables && publish_via_partition_root_given &&
 		!publish_via_partition_root)
 	{
-		/*
-		 * Lock the publication so nobody else can do anything with it. This
-		 * prevents concurrent alter to add partitioned table(s) with WHERE
-		 * clause(s) and/or column lists which we don't allow when not
-		 * publishing via root.
-		 */
-		LockDatabaseObject(PublicationRelationId, pubform->oid, 0,
-						   AccessShareLock);
-
 		root_relids = GetIncludedPublicationRelations(pubform->oid,
 													  PUBLICATION_PART_ROOT);
 
@@ -1659,6 +1650,10 @@ AlterPublication(ParseState *pstate, AlterPublicationStmt *stmt)
 	Relation	rel;
 	HeapTuple	tup;
 	Form_pg_publication pubform;
+	List	   *relations = NIL;
+	List	   *exceptrelations = NIL;
+	List	   *schemaidlist = NIL;
+	Oid			pubid;
 
 	rel = table_open(PublicationRelationId, RowExclusiveLock);
 
@@ -1678,38 +1673,48 @@ AlterPublication(ParseState *pstate, AlterPublicationStmt *stmt)
 		aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_PUBLICATION,
 					   stmt->pubname);
 
-	if (stmt->options)
-		AlterPublicationOptions(pstate, stmt, rel, tup);
-	else
-	{
-		List	   *relations = NIL;
-		List	   *exceptrelations = NIL;
-		List	   *schemaidlist = NIL;
-		Oid			pubid = pubform->oid;
+	pubid = pubform->oid;
 
+	/*
+	 * Resolve publication objects to OIDs when altering the publication
+	 * objects.
+	 */
+	if (!stmt->options)
 		ObjectsInPublicationToOids(stmt->pubobjects, pstate, &relations,
 								   &exceptrelations, &schemaidlist);
 
-		CheckAlterPublication(stmt, tup, relations, schemaidlist);
+	heap_freetuple(tup);
 
-		heap_freetuple(tup);
+	/*
+	 * Lock the publication while we validate and update it. This prevents
+	 * concurrent changes to the publication's relation and schema set, such
+	 * as adding partitioned table(s) with WHERE clause(s) and/or column
+	 * lists, which are not allowed when not publishing via root. It also
+	 * ensures that the publication definition does not change via SET ALL
+	 * TABLES between the validation performed by AlterPublicationOptions()
+	 * and the subsequent catalog update.
+	 */
+	LockDatabaseObject(PublicationRelationId, pubid, 0,
+					   stmt->options ? AccessShareLock : AccessExclusiveLock);
 
-		/* Lock the publication so nobody else can do anything with it. */
-		LockDatabaseObject(PublicationRelationId, pubid, 0,
-						   AccessExclusiveLock);
+	/*
+	 * It is possible that by the time we acquire the lock on publication,
+	 * concurrent DDL has removed it. We can test this by checking the
+	 * existence of publication. We get the tuple again to avoid the risk of
+	 * any publication option getting changed.
+	 */
+	tup = SearchSysCacheCopy1(PUBLICATIONOID, ObjectIdGetDatum(pubid));
+	if (!HeapTupleIsValid(tup))
+		ereport(ERROR,
+				errcode(ERRCODE_UNDEFINED_OBJECT),
+				errmsg("publication \"%s\" does not exist",
+					   stmt->pubname));
 
-		/*
-		 * It is possible that by the time we acquire the lock on publication,
-		 * concurrent DDL has removed it. We can test this by checking the
-		 * existence of publication. We get the tuple again to avoid the risk
-		 * of any publication option getting changed.
-		 */
-		tup = SearchSysCacheCopy1(PUBLICATIONOID, ObjectIdGetDatum(pubid));
-		if (!HeapTupleIsValid(tup))
-			ereport(ERROR,
-					errcode(ERRCODE_UNDEFINED_OBJECT),
-					errmsg("publication \"%s\" does not exist",
-						   stmt->pubname));
+	if (stmt->options)
+		AlterPublicationOptions(pstate, stmt, rel, tup);
+	else
+	{
+		CheckAlterPublication(stmt, tup, relations, schemaidlist);
 
 		relations = list_concat(relations, exceptrelations);
 		AlterPublicationTables(stmt, tup, relations, pstate->p_sourcetext,
-- 
2.55.0