From 5ea4095c44b1911e802161936db1e5dd770927da Mon Sep 17 00:00:00 2001
From: Shlok Kyal <shlok.kyal.oss@gmail.com>
Date: Mon, 24 Feb 2025 15:34:31 +0530
Subject: [PATCH] Selectively invalidate cache in pgoutput

When the ALTER PUBLICATION command is executed, all entries in RelationSyncCache
will be discarded anyway. This mechanism works well but is sometimes not
efficient. For example, when the ALTER PUBLICATION DROP TABLE is executed,
1) the specific entry in RelationSyncCache will be removed, and then 2) all
entries will be discarded.

This patch avoids dropping all entries in RelationSyncCache when ALTER
PUBLICATION is executed. Theoretically, it is enough to invalidate the related
relacache since RelacheCallback has already been registered. The mechanism
exists for ALTER PUBLICATION ADD/SET/DROP, so added for OWNER TO and RENAME
TO statements. Regarding the RENAME TO, we stop using a common function and
replaced it with RenamePublication().

Author: Shlok Kyal and Hayato Kuroda
---
 src/backend/commands/alter.c                |   4 +-
 src/backend/commands/publicationcmds.c      | 107 ++++++++++++++++++++
 src/backend/parser/gram.y                   |   2 +-
 src/backend/replication/pgoutput/pgoutput.c |  18 ----
 src/include/commands/publicationcmds.h      |   1 +
 5 files changed, 112 insertions(+), 20 deletions(-)

diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c
index 78c1d4e1b8..a79329acc1 100644
--- a/src/backend/commands/alter.c
+++ b/src/backend/commands/alter.c
@@ -400,6 +400,9 @@ ExecRenameStmt(RenameStmt *stmt)
 		case OBJECT_TYPE:
 			return RenameType(stmt);
 
+		case OBJECT_PUBLICATION:
+			return RenamePublication(stmt->subname, stmt->newname);
+
 		case OBJECT_AGGREGATE:
 		case OBJECT_COLLATION:
 		case OBJECT_CONVERSION:
@@ -417,7 +420,6 @@ ExecRenameStmt(RenameStmt *stmt)
 		case OBJECT_TSDICTIONARY:
 		case OBJECT_TSPARSER:
 		case OBJECT_TSTEMPLATE:
-		case OBJECT_PUBLICATION:
 		case OBJECT_SUBSCRIPTION:
 			{
 				ObjectAddress address;
diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c
index 150a768d16..182d2187f1 100644
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -491,6 +491,87 @@ pub_contains_invalid_column(Oid pubid, Relation relation, List *ancestors,
 	return *invalid_column_list || *invalid_gen_col;
 }
 
+/*
+ * Execute ALTER PUBLICATION RENAME
+ */
+ObjectAddress
+RenamePublication(const char *oldname, const char *newname)
+{
+	Relation			rel;
+	HeapTuple			tup;
+	ObjectAddress		address;
+	Form_pg_publication	pubform;
+	bool				replaces[Natts_pg_publication];
+	bool				nulls[Natts_pg_publication];
+	Datum				values[Natts_pg_publication];
+
+	rel = table_open(PublicationRelationId, RowExclusiveLock);
+
+	tup = SearchSysCacheCopy1(PUBLICATIONNAME,
+							  CStringGetDatum(oldname));
+
+	if (!HeapTupleIsValid(tup))
+		ereport(ERROR,
+				(errcode(ERRCODE_UNDEFINED_OBJECT),
+				 errmsg("publication \"%s\" does not exist",
+						oldname)));
+
+	pubform = (Form_pg_publication) GETSTRUCT(tup);
+
+	/* must be owner */
+	if (!object_ownercheck(PublicationRelationId, pubform->oid, GetUserId()))
+		aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_PUBLICATION,
+					   NameStr(pubform->pubname));
+
+	/* Everything ok, form a new tuple. */
+	memset(values, 0, sizeof(values));
+	memset(nulls, false, sizeof(nulls));
+	memset(replaces, false, sizeof(replaces));
+
+	/* Only update the pubname */
+	values[Anum_pg_publication_pubname - 1] =
+		DirectFunctionCall1(namein, CStringGetDatum(newname));
+	replaces[Anum_pg_publication_pubname - 1] = true;
+
+	tup = heap_modify_tuple(tup, RelationGetDescr(rel), values, nulls,
+							replaces);
+
+	/* Invalidate the relcache. */
+	if (pubform->puballtables)
+	{
+		CacheInvalidateRelcacheAll();
+	}
+	else
+	{
+		List	   *relids = NIL;
+		List	   *schemarelids = NIL;
+
+		/*
+		 * For partition table, when we insert data, get_rel_sync_entry is
+		 * called and a hash entry is created for the corresponding leaf table.
+		 * So invalidating the leaf nodes would be sufficient here.
+		 */
+		relids = GetPublicationRelations(pubform->oid,
+										 PUBLICATION_PART_LEAF);
+		schemarelids = GetAllSchemaPublicationRelations(pubform->oid,
+														PUBLICATION_PART_LEAF);
+
+		relids = list_concat_unique_oid(relids, schemarelids);
+
+		InvalidatePublicationRels(relids);
+	}
+
+	CatalogTupleUpdate(rel, &tup->t_self, tup);
+
+	ObjectAddressSet(address, PublicationRelationId, pubform->oid);
+
+	heap_freetuple(tup);
+
+	table_close(rel, RowExclusiveLock);
+
+	return address;
+}
+
 /* check_functions_in_node callback */
 static bool
 contain_mutable_or_user_functions_checker(Oid func_id, void *context)
@@ -1996,6 +2077,32 @@ AlterPublicationOwner_internal(Relation rel, HeapTuple tup, Oid newOwnerId)
 	}
 
 	form->pubowner = newOwnerId;
+
+	/* Invalidate the relcache. */
+	if (form->puballtables)
+	{
+		CacheInvalidateRelcacheAll();
+	}
+	else
+	{
+		List	   *relids = NIL;
+		List	   *schemarelids = NIL;
+
+		/*
+		 * For partition table, when we insert data, get_rel_sync_entry is
+		 * called and a hash entry is created for the corresponding leaf table.
+		 * So invalidating the leaf nodes would be sufficient here.
+		 */
+		relids = GetPublicationRelations(form->oid,
+										 PUBLICATION_PART_LEAF);
+		schemarelids = GetAllSchemaPublicationRelations(form->oid,
+														PUBLICATION_PART_LEAF);
+
+		relids = list_concat_unique_oid(relids, schemarelids);
+
+		InvalidatePublicationRels(relids);
+	}
+
 	CatalogTupleUpdate(rel, &tup->t_self, tup);
 
 	/* Update owner dependency reference */
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 7d99c9355c..49fe0567c5 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -9513,7 +9513,7 @@ RenameStmt: ALTER AGGREGATE aggregate_with_argtypes RENAME TO name
 					RenameStmt *n = makeNode(RenameStmt);
 
 					n->renameType = OBJECT_PUBLICATION;
-					n->object = (Node *) makeString($3);
+					n->subname = $3;
 					n->newname = $6;
 					n->missing_ok = false;
 					$$ = (Node *) n;
diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c
index 7d464f656a..b28ce636d5 100644
--- a/src/backend/replication/pgoutput/pgoutput.c
+++ b/src/backend/replication/pgoutput/pgoutput.c
@@ -1789,12 +1789,6 @@ static void
 publication_invalidation_cb(Datum arg, int cacheid, uint32 hashvalue)
 {
 	publications_valid = false;
-
-	/*
-	 * Also invalidate per-relation cache so that next time the filtering info
-	 * is checked it will be updated with the new publication settings.
-	 */
-	rel_sync_cache_publication_cb(arg, cacheid, hashvalue);
 }
 
 /*
@@ -1970,18 +1964,6 @@ init_rel_sync_cache(MemoryContext cachectx)
 								  rel_sync_cache_publication_cb,
 								  (Datum) 0);
 
-	/*
-	 * Flush all cache entries after any publication changes.  (We need no
-	 * callback entry for pg_publication, because publication_invalidation_cb
-	 * will take care of it.)
-	 */
-	CacheRegisterSyscacheCallback(PUBLICATIONRELMAP,
-								  rel_sync_cache_publication_cb,
-								  (Datum) 0);
-	CacheRegisterSyscacheCallback(PUBLICATIONNAMESPACEMAP,
-								  rel_sync_cache_publication_cb,
-								  (Datum) 0);
-
 	relation_callbacks_registered = true;
 }
 
diff --git a/src/include/commands/publicationcmds.h b/src/include/commands/publicationcmds.h
index e11a942ea0..3dfceef70f 100644
--- a/src/include/commands/publicationcmds.h
+++ b/src/include/commands/publicationcmds.h
@@ -38,5 +38,6 @@ extern bool pub_contains_invalid_column(Oid pubid, Relation relation,
 										char pubgencols_type,
 										bool *invalid_column_list,
 										bool *invalid_gen_col);
+extern ObjectAddress RenamePublication(const char *oldname, const char *newname);
 
 #endif							/* PUBLICATIONCMDS_H */
-- 
2.43.5

