v1-0001-Allow-creation-of-publication-with-schema-and-tab.patch

From 63ac94b530bae358d9a781280febe5c8cfa5334f Mon Sep 17 00:00:00 2001
From: Vigneshwaran C <vignesh21@gmail.com>
Date: Mon, 12 Sep 2022 11:24:54 +0530
Subject: [PATCH v1] Allow creation of publication with schema and table of the
 same schema.

Allow creation of publication with schema and table of the same schema.
---
 doc/src/sgml/ref/alter_publication.sgml   | 16 ++--
 doc/src/sgml/ref/create_publication.sgml  | 10 ---
 src/backend/commands/publicationcmds.c    | 99 +----------------------
 src/backend/commands/tablecmds.c          | 27 -------
 src/bin/pg_dump/t/002_pg_dump.pl          | 14 ++++
 src/test/regress/expected/alter_table.out | 14 +++-
 src/test/regress/expected/publication.out | 56 ++++++++++---
 src/test/regress/sql/alter_table.sql      |  3 +-
 src/test/regress/sql/publication.sql      | 19 ++++-
 9 files changed, 94 insertions(+), 164 deletions(-)

diff --git a/doc/src/sgml/ref/alter_publication.sgml b/doc/src/sgml/ref/alter_publication.sgml
index d8ed89ee91..cebedc4058 100644
--- a/doc/src/sgml/ref/alter_publication.sgml
+++ b/doc/src/sgml/ref/alter_publication.sgml
@@ -52,9 +52,11 @@ ALTER PUBLICATION <replaceable class="parameter">name</replaceable> RENAME TO <r
    remove one or more tables/schemas from the publication.  Note that adding
    tables/schemas to a publication that is already subscribed to will require an
    <literal>ALTER SUBSCRIPTION ... REFRESH PUBLICATION</literal> action on the
-   subscribing side in order to become effective. Note also that the combination
-   of <literal>DROP</literal> with a <literal>WHERE</literal> clause is not
-   allowed.
+   subscribing side in order to become effective. Note also that
+   <literal>DROP ALL TABLES IN SCHEMA</literal> will not drop any schema tables
+   that were specified using <literal>FOR TABLE</literal>/
+   <literal>ADD TABLE</literal>, and the combination of <literal>DROP</literal>
+   with a <literal>WHERE</literal> clause is not allowed.
   </para>
 
   <para>
@@ -80,14 +82,6 @@ ALTER PUBLICATION <replaceable class="parameter">name</replaceable> RENAME TO <r
    SCHEMA</literal> publication must be a superuser. However, a superuser can
    change the ownership of a publication regardless of these restrictions.
   </para>
-
-  <para>
-   Adding/Setting a table that is part of schema specified in
-   <literal>ALL TABLES IN SCHEMA</literal>, adding/setting a schema to a
-   publication that already has a table that is part of the specified schema or
-   adding/setting a table to a publication that already has a table's schema as
-   part of the specified schema is not supported.
-  </para>
  </refsect1>
 
  <refsect1>
diff --git a/doc/src/sgml/ref/create_publication.sgml b/doc/src/sgml/ref/create_publication.sgml
index f61641896a..96758af4f7 100644
--- a/doc/src/sgml/ref/create_publication.sgml
+++ b/doc/src/sgml/ref/create_publication.sgml
@@ -109,11 +109,6 @@ CREATE PUBLICATION <replaceable class="parameter">name</replaceable>
       partition are also published via publications that its ancestors are
       part of.
      </para>
-
-     <para>
-      Specifying a table that is part of a schema specified by
-      <literal>FOR ALL TABLES IN SCHEMA</literal> is not supported.
-     </para>
     </listitem>
    </varlistentry>
 
@@ -135,11 +130,6 @@ CREATE PUBLICATION <replaceable class="parameter">name</replaceable>
       the specified list of schemas, including tables created in the future.
      </para>
 
-     <para>
-      Specifying a schema along with a table which belongs to the specified
-      schema using <literal>FOR TABLE</literal> is not supported.
-     </para>
-
      <para>
       Only persistent base tables and partitioned tables present in the schema
       will be included as part of the publication.  Temporary tables, unlogged
diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c
index 8b574b86c4..e36068893d 100644
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -66,7 +66,6 @@ typedef struct rf_context
 	Oid			parentid;		/* relid of the parent relation */
 } rf_context;
 
-static List *OpenRelIdList(List *relids);
 static List *OpenTableList(List *tables);
 static void CloseTableList(List *rels);
 static void LockSchemaList(List *schemalist);
@@ -214,44 +213,6 @@ ObjectsInPublicationToOids(List *pubobjspec_list, ParseState *pstate,
 	}
 }
 
-/*
- * Check if any of the given relation's schema is a member of the given schema
- * list.
- */
-static void
-CheckObjSchemaNotAlreadyInPublication(List *rels, List *schemaidlist,
-									  PublicationObjSpecType checkobjtype)
-{
-	ListCell   *lc;
-
-	foreach(lc, rels)
-	{
-		PublicationRelInfo *pub_rel = (PublicationRelInfo *) lfirst(lc);
-		Relation	rel = pub_rel->relation;
-		Oid			relSchemaId = RelationGetNamespace(rel);
-
-		if (list_member_oid(schemaidlist, relSchemaId))
-		{
-			if (checkobjtype == PUBLICATIONOBJ_TABLES_IN_SCHEMA)
-				ereport(ERROR,
-						errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-						errmsg("cannot add schema \"%s\" to publication",
-							   get_namespace_name(relSchemaId)),
-						errdetail("Table \"%s\" in schema \"%s\" is already part of the publication, adding the same schema is not supported.",
-								  RelationGetRelationName(rel),
-								  get_namespace_name(relSchemaId)));
-			else if (checkobjtype == PUBLICATIONOBJ_TABLE)
-				ereport(ERROR,
-						errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-						errmsg("cannot add relation \"%s.%s\" to publication",
-							   get_namespace_name(relSchemaId),
-							   RelationGetRelationName(rel)),
-						errdetail("Table's schema \"%s\" is already part of the publication or part of the specified schema list.",
-								  get_namespace_name(relSchemaId)));
-		}
-	}
-}
-
 /*
  * Returns true if any of the columns used in the row filter WHERE expression is
  * not part of REPLICA IDENTITY, false otherwise.
@@ -858,9 +819,6 @@ CreatePublication(ParseState *pstate, CreatePublicationStmt *stmt)
 			List	   *rels;
 
 			rels = OpenTableList(relations);
-			CheckObjSchemaNotAlreadyInPublication(rels, schemaidlist,
-												  PUBLICATIONOBJ_TABLE);
-
 			TransformPubWhereClauses(rels, pstate->p_sourcetext,
 									 publish_via_partition_root);
 
@@ -1110,8 +1068,7 @@ InvalidatePublicationRels(List *relids)
  */
 static void
 AlterPublicationTables(AlterPublicationStmt *stmt, HeapTuple tup,
-					   List *tables, List *schemaidlist,
-					   const char *queryString)
+					   List *tables, const char *queryString)
 {
 	List	   *rels = NIL;
 	Form_pg_publication pubform = (Form_pg_publication) GETSTRUCT(tup);
@@ -1129,16 +1086,6 @@ AlterPublicationTables(AlterPublicationStmt *stmt, HeapTuple tup,
 
 	if (stmt->action == AP_AddObjects)
 	{
-		List	   *schemas = NIL;
-
-		/*
-		 * Check if the relation is member of the existing schema in the
-		 * publication or member of the schema list specified.
-		 */
-		schemas = list_concat_copy(schemaidlist, GetPublicationSchemas(pubid));
-		CheckObjSchemaNotAlreadyInPublication(rels, schemas,
-											  PUBLICATIONOBJ_TABLE);
-
 		TransformPubWhereClauses(rels, queryString, pubform->pubviaroot);
 
 		CheckPubRelationColumnList(rels, queryString, pubform->pubviaroot);
@@ -1154,9 +1101,6 @@ AlterPublicationTables(AlterPublicationStmt *stmt, HeapTuple tup,
 		List	   *delrels = NIL;
 		ListCell   *oldlc;
 
-		CheckObjSchemaNotAlreadyInPublication(rels, schemaidlist,
-											  PUBLICATIONOBJ_TABLE);
-
 		TransformPubWhereClauses(rels, queryString, pubform->pubviaroot);
 
 		CheckPubRelationColumnList(rels, queryString, pubform->pubviaroot);
@@ -1307,19 +1251,7 @@ AlterPublicationSchemas(AlterPublicationStmt *stmt,
 	 */
 	LockSchemaList(schemaidlist);
 	if (stmt->action == AP_AddObjects)
-	{
-		List	   *rels;
-		List	   *reloids;
-
-		reloids = GetPublicationRelations(pubform->oid, PUBLICATION_PART_ROOT);
-		rels = OpenRelIdList(reloids);
-
-		CheckObjSchemaNotAlreadyInPublication(rels, schemaidlist,
-											  PUBLICATIONOBJ_TABLES_IN_SCHEMA);
-
-		CloseTableList(rels);
 		PublicationAddSchemas(pubform->oid, schemaidlist, false, stmt);
-	}
 	else if (stmt->action == AP_DropObjects)
 		PublicationDropSchemas(pubform->oid, schemaidlist, false);
 	else						/* AP_SetObjects */
@@ -1453,8 +1385,7 @@ AlterPublication(ParseState *pstate, AlterPublicationStmt *stmt)
 					errmsg("publication \"%s\" does not exist",
 						   stmt->pubname));
 
-		AlterPublicationTables(stmt, tup, relations, schemaidlist,
-							   pstate->p_sourcetext);
+		AlterPublicationTables(stmt, tup, relations, pstate->p_sourcetext);
 		AlterPublicationSchemas(stmt, tup, schemaidlist);
 	}
 
@@ -1569,32 +1500,6 @@ RemovePublicationSchemaById(Oid psoid)
 	table_close(rel, RowExclusiveLock);
 }
 
-/*
- * Open relations specified by a relid list.
- * The returned tables are locked in ShareUpdateExclusiveLock mode in order to
- * add them to a publication.
- */
-static List *
-OpenRelIdList(List *relids)
-{
-	ListCell   *lc;
-	List	   *rels = NIL;
-
-	foreach(lc, relids)
-	{
-		PublicationRelInfo *pub_rel;
-		Oid			relid = lfirst_oid(lc);
-		Relation	rel = table_open(relid,
-									 ShareUpdateExclusiveLock);
-
-		pub_rel = palloc(sizeof(PublicationRelInfo));
-		pub_rel->relation = rel;
-		rels = lappend(rels, pub_rel);
-	}
-
-	return rels;
-}
-
 /*
  * Open relations specified by a PublicationTable list.
  * The returned tables are locked in ShareUpdateExclusiveLock mode in order to
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 53b0f3a9c1..ac95e18e71 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -16397,33 +16397,6 @@ AlterTableNamespace(AlterObjectSchemaStmt *stmt, Oid *oldschema)
 	newrv = makeRangeVar(stmt->newschema, RelationGetRelationName(rel), -1);
 	nspOid = RangeVarGetAndCheckCreationNamespace(newrv, NoLock, NULL);
 
-	/*
-	 * Check that setting the relation to a different schema won't result in a
-	 * publication having both a schema and the same schema's table, as this
-	 * is not supported.
-	 */
-	if (stmt->objectType == OBJECT_TABLE)
-	{
-		ListCell   *lc;
-		List	   *schemaPubids = GetSchemaPublications(nspOid);
-		List	   *relPubids = GetRelationPublications(RelationGetRelid(rel));
-
-		foreach(lc, relPubids)
-		{
-			Oid			pubid = lfirst_oid(lc);
-
-			if (list_member_oid(schemaPubids, pubid))
-				ereport(ERROR,
-						errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-						errmsg("cannot move table \"%s\" to schema \"%s\"",
-							   RelationGetRelationName(rel), stmt->newschema),
-						errdetail("The schema \"%s\" and same schema's table \"%s\" cannot be part of the same publication \"%s\".",
-								  stmt->newschema,
-								  RelationGetRelationName(rel),
-								  get_publication_name(pubid, false)));
-		}
-	}
-
 	/* common checks on switching namespaces */
 	CheckSetNamespace(oldNspOid, nspOid);
 
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 2873b662fb..714b1321ae 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -2565,6 +2565,20 @@ my %tests = (
 		like => { %full_runs, section_post_data => 1, },
 	},
 
+	'ALTER PUBLICATION pub3 ADD TABLE test_table' => {
+		create_order => 51,
+		create_sql =>
+		  'ALTER PUBLICATION pub3 ADD TABLE dump_test.test_table;',
+		regexp => qr/^
+			\QALTER PUBLICATION pub3 ADD TABLE ONLY dump_test.test_table;\E
+			/xm,
+		like   => { %full_runs, section_post_data => 1, },
+		unlike => {
+			exclude_dump_test_schema => 1,
+			exclude_test_table       => 1,
+		},
+	},
+
 	'ALTER PUBLICATION pub4 ADD TABLE test_table WHERE (col1 > 0);' => {
 		create_order => 51,
 		create_sql =>
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index d63f4f1cba..0906c080b5 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -4595,10 +4595,16 @@ create table alter1.t1 (a int);
 set client_min_messages = 'ERROR';
 create publication pub1 for table alter1.t1, all tables in schema alter2;
 reset client_min_messages;
-alter table alter1.t1 set schema alter2; -- should fail
-ERROR:  cannot move table "t1" to schema "alter2"
-DETAIL:  The schema "alter2" and same schema's table "t1" cannot be part of the same publication "pub1".
+alter table alter1.t1 set schema alter2;
+\d+ alter2.t1
+                                    Table "alter2.t1"
+ Column |  Type   | Collation | Nullable | Default | Storage | Stats target | Description 
+--------+---------+-----------+----------+---------+---------+--------------+-------------
+ a      | integer |           |          |         | plain   |              | 
+Publications:
+    "pub1"
+
 drop publication pub1;
 drop schema alter1 cascade;
-NOTICE:  drop cascades to table alter1.t1
 drop schema alter2 cascade;
+NOTICE:  drop cascades to table alter2.t1
diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out
index e6e082de2f..eb0c1373c9 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -118,15 +118,42 @@ Tables from schemas:
 
 SET client_min_messages = 'ERROR';
 CREATE PUBLICATION testpub_forschema FOR ALL TABLES IN SCHEMA pub_test;
-RESET client_min_messages;
--- fail - can't create publication with schema and table of the same schema
+-- should be able to create publication with schema and table of the same
+-- schema
 CREATE PUBLICATION testpub_for_tbl_schema FOR ALL TABLES IN SCHEMA pub_test, TABLE pub_test.testpub_nopk;
-ERROR:  cannot add relation "pub_test.testpub_nopk" to publication
-DETAIL:  Table's schema "pub_test" is already part of the publication or part of the specified schema list.
--- fail - can't add a table of the same schema to the schema publication
+RESET client_min_messages;
+\dRp+ testpub_for_tbl_schema
+                             Publication testpub_for_tbl_schema
+          Owner           | All tables | Inserts | Updates | Deletes | Truncates | Via root 
+--------------------------+------------+---------+---------+---------+-----------+----------
+ regress_publication_user | f          | t       | t       | t       | t         | f
+Tables:
+    "pub_test.testpub_nopk"
+Tables from schemas:
+    "pub_test"
+
+-- should be able to add a table of the same schema to the schema publication
 ALTER PUBLICATION testpub_forschema ADD TABLE pub_test.testpub_nopk;
-ERROR:  cannot add relation "pub_test.testpub_nopk" to publication
-DETAIL:  Table's schema "pub_test" is already part of the publication or part of the specified schema list.
+\dRp+ testpub_forschema
+                               Publication testpub_forschema
+          Owner           | All tables | Inserts | Updates | Deletes | Truncates | Via root 
+--------------------------+------------+---------+---------+---------+-----------+----------
+ regress_publication_user | f          | t       | t       | t       | t         | f
+Tables:
+    "pub_test.testpub_nopk"
+Tables from schemas:
+    "pub_test"
+
+-- should be able to drop the table
+ALTER PUBLICATION testpub_forschema DROP TABLE pub_test.testpub_nopk;
+\dRp+ testpub_forschema
+                               Publication testpub_forschema
+          Owner           | All tables | Inserts | Updates | Deletes | Truncates | Via root 
+--------------------------+------------+---------+---------+---------+-----------+----------
+ regress_publication_user | f          | t       | t       | t       | t         | f
+Tables from schemas:
+    "pub_test"
+
 -- fail - can't drop a table from the schema publication which isn't in the
 -- publication
 ALTER PUBLICATION testpub_forschema DROP TABLE pub_test.testpub_nopk;
@@ -166,7 +193,7 @@ Publications:
 (1 row)
 
 DROP TABLE testpub_tbl2;
-DROP PUBLICATION testpub_foralltables, testpub_fortable, testpub_forschema;
+DROP PUBLICATION testpub_foralltables, testpub_fortable, testpub_forschema, testpub_for_tbl_schema;
 CREATE TABLE testpub_tbl3 (a int);
 CREATE TABLE testpub_tbl3a (b text) INHERITS (testpub_tbl3);
 SET client_min_messages = 'ERROR';
@@ -466,10 +493,19 @@ ERROR:  cannot use a WHERE clause when removing a table from a publication
 -- fail - cannot ALTER SET table which is a member of a pre-existing schema
 SET client_min_messages = 'ERROR';
 CREATE PUBLICATION testpub6 FOR ALL TABLES IN SCHEMA testpub_rf_schema2;
+-- should be able to set publication with schema and table of the same schema
 ALTER PUBLICATION testpub6 SET ALL TABLES IN SCHEMA testpub_rf_schema2, TABLE testpub_rf_schema2.testpub_rf_tbl6 WHERE (i < 99);
-ERROR:  cannot add relation "testpub_rf_schema2.testpub_rf_tbl6" to publication
-DETAIL:  Table's schema "testpub_rf_schema2" is already part of the publication or part of the specified schema list.
 RESET client_min_messages;
+\dRp+ testpub6
+                                    Publication testpub6
+          Owner           | All tables | Inserts | Updates | Deletes | Truncates | Via root 
+--------------------------+------------+---------+---------+---------+-----------+----------
+ regress_publication_user | f          | t       | t       | t       | t         | f
+Tables:
+    "testpub_rf_schema2.testpub_rf_tbl6" WHERE (i < 99)
+Tables from schemas:
+    "testpub_rf_schema2"
+
 DROP TABLE testpub_rf_tbl1;
 DROP TABLE testpub_rf_tbl2;
 DROP TABLE testpub_rf_tbl3;
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index e7013f5e15..3f01fdd8a8 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -3031,7 +3031,8 @@ create table alter1.t1 (a int);
 set client_min_messages = 'ERROR';
 create publication pub1 for table alter1.t1, all tables in schema alter2;
 reset client_min_messages;
-alter table alter1.t1 set schema alter2; -- should fail
+alter table alter1.t1 set schema alter2;
+\d+ alter2.t1
 drop publication pub1;
 drop schema alter1 cascade;
 drop schema alter2 cascade;
diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql
index a56387edee..ff1d49ec10 100644
--- a/src/test/regress/sql/publication.sql
+++ b/src/test/regress/sql/publication.sql
@@ -73,11 +73,20 @@ ALTER PUBLICATION testpub_fortable SET ALL TABLES IN SCHEMA pub_test;
 
 SET client_min_messages = 'ERROR';
 CREATE PUBLICATION testpub_forschema FOR ALL TABLES IN SCHEMA pub_test;
-RESET client_min_messages;
--- fail - can't create publication with schema and table of the same schema
+-- should be able to create publication with schema and table of the same
+-- schema
 CREATE PUBLICATION testpub_for_tbl_schema FOR ALL TABLES IN SCHEMA pub_test, TABLE pub_test.testpub_nopk;
--- fail - can't add a table of the same schema to the schema publication
+RESET client_min_messages;
+\dRp+ testpub_for_tbl_schema
+
+-- should be able to add a table of the same schema to the schema publication
 ALTER PUBLICATION testpub_forschema ADD TABLE pub_test.testpub_nopk;
+\dRp+ testpub_forschema
+
+-- should be able to drop the table
+ALTER PUBLICATION testpub_forschema DROP TABLE pub_test.testpub_nopk;
+\dRp+ testpub_forschema
+
 -- fail - can't drop a table from the schema publication which isn't in the
 -- publication
 ALTER PUBLICATION testpub_forschema DROP TABLE pub_test.testpub_nopk;
@@ -90,7 +99,7 @@ SELECT pubname, puballtables FROM pg_publication WHERE pubname = 'testpub_forall
 \dRp+ testpub_foralltables
 
 DROP TABLE testpub_tbl2;
-DROP PUBLICATION testpub_foralltables, testpub_fortable, testpub_forschema;
+DROP PUBLICATION testpub_foralltables, testpub_fortable, testpub_forschema, testpub_for_tbl_schema;
 
 CREATE TABLE testpub_tbl3 (a int);
 CREATE TABLE testpub_tbl3a (b text) INHERITS (testpub_tbl3);
@@ -242,8 +251,10 @@ ALTER PUBLICATION testpub5 DROP TABLE testpub_rf_tbl1 WHERE (e < 27);
 -- fail - cannot ALTER SET table which is a member of a pre-existing schema
 SET client_min_messages = 'ERROR';
 CREATE PUBLICATION testpub6 FOR ALL TABLES IN SCHEMA testpub_rf_schema2;
+-- should be able to set publication with schema and table of the same schema
 ALTER PUBLICATION testpub6 SET ALL TABLES IN SCHEMA testpub_rf_schema2, TABLE testpub_rf_schema2.testpub_rf_tbl6 WHERE (i < 99);
 RESET client_min_messages;
+\dRp+ testpub6
 
 DROP TABLE testpub_rf_tbl1;
 DROP TABLE testpub_rf_tbl2;
-- 
2.32.0