From 631a6d04cbe420164833dd4e88a86d0e076fd47d Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Tue, 12 Apr 2022 12:59:50 +0200
Subject: [PATCH v2] fixup checking for rowfilter/collist on altering
 publication

---
 src/backend/commands/publicationcmds.c    | 91 +++++++++++------------
 src/test/regress/expected/publication.out |  8 +-
 2 files changed, 49 insertions(+), 50 deletions(-)

diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c
index 7aacb6b2fe..59fc39e9f2 100644
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -945,60 +945,59 @@ AlterPublicationOptions(ParseState *pstate, AlterPublicationStmt *stmt,
 
 		foreach(lc, root_relids)
 		{
-			HeapTuple	rftuple;
 			Oid			relid = lfirst_oid(lc);
-			bool		has_column_list;
-			bool		has_row_filter;
+			char		relkind;
+			char	   *relname;
+			HeapTuple	rftuple;
+			bool		has_rowfilter;
+			bool		has_collist;
+
+			/* Beware: we don't have lock on the relations */
 
 			rftuple = SearchSysCache2(PUBLICATIONRELMAP,
 									  ObjectIdGetDatum(relid),
 									  ObjectIdGetDatum(pubform->oid));
-
-			has_row_filter
-				= !heap_attisnull(rftuple, Anum_pg_publication_rel_prqual, NULL);
-
-			has_column_list
-				= !heap_attisnull(rftuple, Anum_pg_publication_rel_prattrs, NULL);
-
-			if (HeapTupleIsValid(rftuple) &&
-				(has_row_filter || has_column_list))
+			if (!HeapTupleIsValid(rftuple))
+				continue;
+			has_rowfilter = !heap_attisnull(rftuple, Anum_pg_publication_rel_prqual, NULL);
+			has_collist = !heap_attisnull(rftuple, Anum_pg_publication_rel_prattrs, NULL);
+			if (!has_rowfilter && !has_collist)
 			{
-				HeapTuple	tuple;
-
-				tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
-				if (HeapTupleIsValid(tuple))
-				{
-					Form_pg_class relform = (Form_pg_class) GETSTRUCT(tuple);
-
-					if ((relform->relkind == RELKIND_PARTITIONED_TABLE) &&
-						has_row_filter)
-						ereport(ERROR,
-								(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-								 errmsg("cannot set %s for publication \"%s\"",
-										"publish_via_partition_root = false",
-										stmt->pubname),
-								 errdetail("The publication contains a WHERE clause for a partitioned table \"%s\" "
-										   "which is not allowed when %s is false.",
-										   NameStr(relform->relname),
-										   "publish_via_partition_root")));
-
-					if ((relform->relkind == RELKIND_PARTITIONED_TABLE) &&
-						has_column_list)
-						ereport(ERROR,
-								(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-								 errmsg("cannot set %s for publication \"%s\"",
-										"publish_via_partition_root = false",
-										stmt->pubname),
-								 errdetail("The publication contains a column list for a partitioned table \"%s\" "
-										   "which is not allowed when %s is false.",
-										   NameStr(relform->relname),
-										   "publish_via_partition_root")));
-
-					ReleaseSysCache(tuple);
-				}
-
 				ReleaseSysCache(rftuple);
+				continue;
 			}
+
+			relkind = get_rel_relkind(relid);
+			if (relkind != RELKIND_PARTITIONED_TABLE)
+			{
+				ReleaseSysCache(rftuple);
+				continue;
+			}
+			relname = get_rel_name(relid);
+			if (relname == NULL)	/* table concurrently dropped */
+			{
+				ReleaseSysCache(rftuple);
+				continue;
+			}
+
+			if (has_rowfilter)
+				ereport(ERROR,
+						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+						 errmsg("cannot set parameter \"%s\" to false for publication \"%s\"",
+								"publish_via_partition_root",
+								stmt->pubname),
+						 errdetail("The publication includes partitioned table \"%s\" with a WHERE clause, which is not allowed when \"%s\" is false.",
+								   get_rel_name(relid),
+								   "publish_via_partition_root")));
+			Assert(has_collist);
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+					 errmsg("cannot set parameter \"%s\" to false for publication \"%s\"",
+							"publish_via_partition_root",
+							stmt->pubname),
+					 errdetail("The publication includes partitioned table \"%s\" with a column list, which is not allowed when \"%s\" is false.",
+							   get_rel_name(relid),
+							   "publish_via_partition_root")));
 		}
 	}
 
diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out
index 8208f9fa0e..a6fd4f6c89 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -588,8 +588,8 @@ UPDATE rf_tbl_abcd_part_pk SET a = 1;
 -- fail - cannot set PUBLISH_VIA_PARTITION_ROOT to false if any row filter is
 -- used for partitioned table
 ALTER PUBLICATION testpub6 SET (PUBLISH_VIA_PARTITION_ROOT=0);
-ERROR:  cannot set publish_via_partition_root = false for publication "testpub6"
-DETAIL:  The publication contains a WHERE clause for a partitioned table "rf_tbl_abcd_part_pk" which is not allowed when publish_via_partition_root is false.
+ERROR:  cannot set parameter "publish_via_partition_root" to false for publication "testpub6"
+DETAIL:  The publication includes partitioned table "rf_tbl_abcd_part_pk" with a WHERE clause, which is not allowed when "publish_via_partition_root" is false.
 -- Now change the root filter to use a column "b"
 -- (which is not in the replica identity)
 ALTER PUBLICATION testpub6 SET TABLE rf_tbl_abcd_part_pk_1 WHERE (b > 99);
@@ -951,8 +951,8 @@ UPDATE rf_tbl_abcd_part_pk SET a = 1;
 -- fail - cannot set PUBLISH_VIA_PARTITION_ROOT to false if any column list is
 -- used for partitioned table
 ALTER PUBLICATION testpub6 SET (PUBLISH_VIA_PARTITION_ROOT=0);
-ERROR:  cannot set publish_via_partition_root = false for publication "testpub6"
-DETAIL:  The publication contains a column list for a partitioned table "rf_tbl_abcd_part_pk" which is not allowed when publish_via_partition_root is false.
+ERROR:  cannot set parameter "publish_via_partition_root" to false for publication "testpub6"
+DETAIL:  The publication includes partitioned table "rf_tbl_abcd_part_pk" with a column list, which is not allowed when "publish_via_partition_root" is false.
 -- Now change the root column list to use a column "b"
 -- (which is not in the replica identity)
 ALTER PUBLICATION testpub6 SET TABLE rf_tbl_abcd_part_pk_1 (b);
-- 
2.30.2

