v3-0001-fix-new-collist-validation-msg.patch

From 0f3e3786a7619650e817b6e212d865361b2b06c9 Mon Sep 17 00:00:00 2001
From: Peter Smith <peter.b.smith@fujitsu.com>
Date: Fri, 2 Aug 2024 12:42:17 +1000
Subject: [PATCH v3] fix new collist validation msg

---
 src/backend/catalog/pg_publication.c      | 26 +++++++++++++++-----------
 src/backend/commands/publicationcmds.c    | 20 +++++++++-----------
 src/include/catalog/pg_publication.h      |  3 +++
 src/test/regress/expected/publication.out |  7 +++++++
 src/test/regress/sql/publication.sql      |  4 ++++
 5 files changed, 38 insertions(+), 22 deletions(-)

diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c
index 0602398..bec11ee 100644
--- a/src/backend/catalog/pg_publication.c
+++ b/src/backend/catalog/pg_publication.c
@@ -48,9 +48,6 @@ typedef struct
 								 * table. */
 } published_rel;
 
-static void publication_translate_columns(Relation targetrel, List *columns,
-										  int *natts, AttrNumber **attrs);
-
 /*
  * Check if relation can be in given publication and throws appropriate
  * error if not.
@@ -401,7 +398,7 @@ publication_add_relation(Oid pubid, PublicationRelInfo *pri,
 	 * Also build an array of attnums, for storing in the catalog.
 	 */
 	publication_translate_columns(pri->relation, pri->columns,
-								  &natts, &attarray);
+								  NULL, &natts, &attarray);
 
 	/* Form a tuple. */
 	memset(values, 0, sizeof(values));
@@ -488,18 +485,21 @@ compare_int16(const void *a, const void *b)
 }
 
 /*
- * Translate a list of column names to an array of attribute numbers
- * and a Bitmapset with them; verify that each attribute is appropriate
- * to have in a publication column list (no system or generated attributes,
- * no duplicates).  Additional checks with replica identity are done later;
- * see pub_collist_contains_invalid_column.
+ * Translate a list of column names to an array of attribute numbers.
+ * The equivalent Bitmapset is also returned when bms_attrs is specified.
+ *
+ * Verify that each attribute is appropriate to have in a
+ * publication column list (no system or generated attributes, no duplicates).
+ * Additional checks with replica identity are done later; see
+ * pub_collist_contains_invalid_column.
  *
  * Note that the attribute numbers are *not* offset by
  * FirstLowInvalidHeapAttributeNumber; system columns are forbidden so this
  * is okay.
  */
-static void
+void
 publication_translate_columns(Relation targetrel, List *columns,
+							  Bitmapset **bms_attrs,
 							  int *natts, AttrNumber **attrs)
 {
 	AttrNumber *attarray = NULL;
@@ -508,6 +508,9 @@ publication_translate_columns(Relation targetrel, List *columns,
 	int			n = 0;
 	TupleDesc	tupdesc = RelationGetDescr(targetrel);
 
+	if (bms_attrs)
+		*bms_attrs = set;
+
 	/* Bail out when no column list defined. */
 	if (!columns)
 		return;
@@ -556,7 +559,8 @@ publication_translate_columns(Relation targetrel, List *columns,
 	*natts = n;
 	*attrs = attarray;
 
-	bms_free(set);
+	if (!bms_attrs)
+		bms_free(set);
 }
 
 /*
diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c
index 6ea7099..e7e6977 100644
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -1176,20 +1176,18 @@ AlterPublicationTables(AlterPublicationStmt *stmt, HeapTuple tup,
 				newrelid = RelationGetRelid(newpubrel->relation);
 
 				/*
-				 * If the new publication has column list, transform it to a
-				 * bitmap too.
+				 * If the new publication has column list, validate it
+				 * before transforming it to a bitmap too.
 				 */
 				if (newpubrel->columns)
 				{
-					ListCell   *lc;
+					AttrNumber *attarray = NULL;
+					int			natts = 0;
 
-					foreach(lc, newpubrel->columns)
-					{
-						char	   *colname = strVal(lfirst(lc));
-						AttrNumber	attnum = get_attnum(newrelid, colname);
-
-						newcolumns = bms_add_member(newcolumns, attnum);
-					}
+					publication_translate_columns(newpubrel->relation,
+												  newpubrel->columns,
+												  &newcolumns,
+												  &natts, &attarray);
 				}
 
 				/*
@@ -1199,7 +1197,7 @@ AlterPublicationTables(AlterPublicationStmt *stmt, HeapTuple tup,
 				 * expressions also match. Same for the column list. Drop the
 				 * rest.
 				 */
-				if (RelationGetRelid(newpubrel->relation) == oldrelid)
+				if (newrelid == oldrelid)
 				{
 					if (equal(oldrelwhereclause, newpubrel->whereClause) &&
 						bms_equal(oldcolumns, newcolumns))
diff --git a/src/include/catalog/pg_publication.h b/src/include/catalog/pg_publication.h
index 2f1b6ab..5f99d9d 100644
--- a/src/include/catalog/pg_publication.h
+++ b/src/include/catalog/pg_publication.h
@@ -157,5 +157,8 @@ extern ObjectAddress publication_add_schema(Oid pubid, Oid schemaid,
 
 extern Bitmapset *pub_collist_to_bitmapset(Bitmapset *columns, Datum pubcols,
 										   MemoryContext mcxt);
+extern void publication_translate_columns(Relation targetrel, List *columns,
+										  Bitmapset **bms_attrs,
+										  int *natts, AttrNumber **attrs);
 
 #endif							/* PG_PUBLICATION_H */
diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out
index 30b6371..660245e 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -693,6 +693,13 @@ ERROR:  cannot use generated column "d" in publication column list
 -- error: system attributes "ctid" not allowed in column list
 ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, ctid);
 ERROR:  cannot use system column "ctid" in publication column list
+ALTER PUBLICATION testpub_fortable SET TABLE testpub_tbl1 (id, ctid);
+ERROR:  cannot use system column "ctid" in publication column list
+-- error: duplicates not allowed in column list
+ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, a);
+ERROR:  duplicate column "a" in publication column list
+ALTER PUBLICATION testpub_fortable SET TABLE testpub_tbl5 (a, a);
+ERROR:  duplicate column "a" in publication column list
 -- ok
 ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, c);
 ALTER TABLE testpub_tbl5 DROP COLUMN c;		-- no dice
diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql
index 479d4f3..f68a5b5 100644
--- a/src/test/regress/sql/publication.sql
+++ b/src/test/regress/sql/publication.sql
@@ -417,6 +417,10 @@ ALTER PUBLICATION testpub_fortable DROP TABLE testpub_tbl5;
 ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, d);
 -- error: system attributes "ctid" not allowed in column list
 ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, ctid);
+ALTER PUBLICATION testpub_fortable SET TABLE testpub_tbl1 (id, ctid);
+-- error: duplicates not allowed in column list
+ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, a);
+ALTER PUBLICATION testpub_fortable SET TABLE testpub_tbl5 (a, a);
 -- ok
 ALTER PUBLICATION testpub_fortable ADD TABLE testpub_tbl5 (a, c);
 ALTER TABLE testpub_tbl5 DROP COLUMN c;		-- no dice
-- 
1.8.3.1