[CF 7210] v1 - Publication of same parent table with ONLY-ness mismatch should error

This branch was automatically generated by a robot using patches from an
email thread registered at:

https://commitfest.postgresql.org/patch/7210

The branch will be overwritten each time a new patch version is posted to
the thread, and also periodically to check for bitrot caused by changes
on the master branch.

Patch(es): https://www.postgresql.org/message-id/CAHut+PtKp7OFkrvg8kJxH00MKCDEJRBpAzrzWE8oz84rq5AKbg@mail.gmail.com
Author(s): Peter Smith

Author: Commitfest Bot

doc/src/sgml/ref/create_publication.sgml

@@ -119,9 +119,9 @@ CREATE PUBLICATION <replaceable class="parameter">name</replaceable>
       Optionally, <literal>*</literal> can be specified after the
       <replaceable class="parameter">table_name</replaceable> to
       explicitly indicate that descendant tables are included.
-      This does not apply to a partitioned table, however.  The partitions of
-      a partitioned table are always implicitly considered part of the
-      publication, so they are never explicitly added to the publication.
+      For a partitioned table, <literal>ONLY</literal> and <literal>*</literal>
+      have no effect: its partitions are always implicitly considered part of
+      the publication, so they are never explicitly added to the publication.
      </para>
 
      <para>

src/backend/commands/publicationcmds.c

@@ -1847,6 +1847,7 @@ OpenTableList(List *tables)
 	ListCell   *lc;
 	List	   *relids_with_rf = NIL;
 	List	   *relids_with_collist = NIL;
+	List	   *relids_recursing = NIL;
 
 	/*
 	 * Open, share-lock, and check all the explicitly-specified relations
@@ -1888,6 +1889,22 @@ OpenTableList(List *tables)
 						 errmsg("conflicting or redundant column lists for table \"%s\"",
 								RelationGetRelationName(rel))));
 
+			/*
+			 * Disallow duplicate tables if one mention specifies ONLY and
+			 * another does not, since it's not clear whether descendant
+			 * tables should be included.  Plain "foo, foo" (agreeing on
+			 * ONLY-ness) remains a silently-tolerated no-op, as before.
+			 *
+			 * Mismatching ONLY-ness remains silently-tolerated for
+			 * partitioned tables since ONLY has no meaning for them anyway.
+			 */
+			if (recurse != list_member_oid(relids_recursing, myrelid) &&
+				rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE)
+				ereport(ERROR,
+						(errcode(ERRCODE_DUPLICATE_OBJECT),
+						 errmsg("conflicting ONLY specifications for table \"%s\"",
+								RelationGetRelationName(rel))));
+
 			table_close(rel, ShareUpdateExclusiveLock);
 			continue;
 		}
@@ -1906,6 +1923,9 @@ OpenTableList(List *tables)
 		if (t->columns)
 			relids_with_collist = lappend_oid(relids_with_collist, myrelid);
 
+		if (recurse)
+			relids_recursing = lappend_oid(relids_recursing, myrelid);
+
 		/*
 		 * Add children of this rel, if requested, so that they too are added
 		 * to the publication.  A partitioned table can't have any inheritance

src/test/regress/expected/publication.out

@@ -474,9 +474,38 @@ CREATE PUBLICATION testpub7 FOR ALL TABLES EXCEPT (TABLE ONLY testpub_tbl_parent
 Except tables:
     "public.testpub_tbl_parent"
 
+-- Listing the same table twice is fine as long as the mentions agree on
+-- ONLY-ness.
+CREATE PUBLICATION testpub_dup1 FOR TABLE testpub_tbl_parent, TABLE testpub_tbl_parent;
+\dRp+ testpub_dup1
+                                                           Publication testpub_dup1
+          Owner           | All tables | All sequences | Inserts | Updates | Deletes | Truncates | Generated columns | Via root | Description 
+--------------------------+------------+---------------+---------+---------+---------+-----------+-------------------+----------+-------------
+ regress_publication_user | f          | f             | t       | t       | t       | t         | none              | f        | 
+Tables:
+    "public.testpub_tbl_child"
+    "public.testpub_tbl_parent"
+
+CREATE PUBLICATION testpub_dup2 FOR TABLE ONLY testpub_tbl_parent, TABLE ONLY testpub_tbl_parent;
+\dRp+ testpub_dup2
+                                                           Publication testpub_dup2
+          Owner           | All tables | All sequences | Inserts | Updates | Deletes | Truncates | Generated columns | Via root | Description 
+--------------------------+------------+---------------+---------+---------+---------+-----------+-------------------+----------+-------------
+ regress_publication_user | f          | f             | t       | t       | t       | t         | none              | f        | 
+Tables:
+    "public.testpub_tbl_parent"
+
+-- But mixing ONLY and non-ONLY mentions of the same table is ambiguous
+-- (would descendants be included or not?) and must be rejected.
 RESET client_min_messages;
+CREATE PUBLICATION testpub_conflict1 FOR TABLE testpub_tbl_parent, TABLE ONLY testpub_tbl_parent;
+ERROR:  conflicting ONLY specifications for table "testpub_tbl_parent"
+CREATE PUBLICATION testpub_conflict2 FOR TABLE ONLY testpub_tbl_parent, TABLE testpub_tbl_parent;
+ERROR:  conflicting ONLY specifications for table "testpub_tbl_parent"
+SET client_min_messages = 'ERROR';
 DROP TABLE testpub_tbl_parent, testpub_tbl_child;
 DROP PUBLICATION testpub3, testpub4, testpub5, testpub6, testpub7;
+DROP PUBLICATION testpub_dup1, testpub_dup2;
 ---------------------------------------------
 -- EXCEPT clause tests for partitioned tables
 ---------------------------------------------
@@ -656,8 +685,28 @@ HINT:  To enable updating the table, set REPLICA IDENTITY using ALTER TABLE.
 ALTER PUBLICATION testpub_forparted DROP TABLE testpub_parted;
 -- works again, because update is no longer replicated
 UPDATE testpub_parted2 SET a = 2;
+-- Mixing ONLY and non-ONLY mentions of the same partitioned table is silently
+-- tolerated because ONLY has no effect for partitioned tables anyway.
+CREATE PUBLICATION testpub_dup1 FOR TABLE testpub_parted, TABLE ONLY testpub_parted;
+\dRp+ testpub_dup1
+                                                           Publication testpub_dup1
+          Owner           | All tables | All sequences | Inserts | Updates | Deletes | Truncates | Generated columns | Via root | Description 
+--------------------------+------------+---------------+---------+---------+---------+-----------+-------------------+----------+-------------
+ regress_publication_user | f          | f             | t       | t       | t       | t         | none              | f        | 
+Tables:
+    "public.testpub_parted"
+
+CREATE PUBLICATION testpub_dup2 FOR TABLE ONLY testpub_parted, TABLE testpub_parted;
+\dRp+ testpub_dup2
+                                                           Publication testpub_dup2
+          Owner           | All tables | All sequences | Inserts | Updates | Deletes | Truncates | Generated columns | Via root | Description 
+--------------------------+------------+---------------+---------+---------+---------+-----------+-------------------+----------+-------------
+ regress_publication_user | f          | f             | t       | t       | t       | t         | none              | f        | 
+Tables:
+    "public.testpub_parted"
+
 DROP TABLE testpub_parted1, testpub_parted2;
-DROP PUBLICATION testpub_forparted, testpub_forparted1;
+DROP PUBLICATION testpub_forparted, testpub_forparted1, testpub_dup1, testpub_dup2;
 -- Tests for row filters
 CREATE TABLE testpub_rf_tbl1 (a integer, b text);
 CREATE TABLE testpub_rf_tbl2 (c text, d integer);

src/test/regress/sql/publication.sql

@@ -234,9 +234,23 @@ CREATE PUBLICATION testpub6 FOR ALL TABLES EXCEPT (TABLE testpub_tbl_parent *);
 CREATE PUBLICATION testpub7 FOR ALL TABLES EXCEPT (TABLE ONLY testpub_tbl_parent);
 \dRp+ testpub7
 
+-- Listing the same table twice is fine as long as the mentions agree on
+-- ONLY-ness.
+CREATE PUBLICATION testpub_dup1 FOR TABLE testpub_tbl_parent, TABLE testpub_tbl_parent;
+\dRp+ testpub_dup1
+CREATE PUBLICATION testpub_dup2 FOR TABLE ONLY testpub_tbl_parent, TABLE ONLY testpub_tbl_parent;
+\dRp+ testpub_dup2
+
+-- But mixing ONLY and non-ONLY mentions of the same table is ambiguous
+-- (would descendants be included or not?) and must be rejected.
 RESET client_min_messages;
+CREATE PUBLICATION testpub_conflict1 FOR TABLE testpub_tbl_parent, TABLE ONLY testpub_tbl_parent;
+CREATE PUBLICATION testpub_conflict2 FOR TABLE ONLY testpub_tbl_parent, TABLE testpub_tbl_parent;
+
+SET client_min_messages = 'ERROR';
 DROP TABLE testpub_tbl_parent, testpub_tbl_child;
 DROP PUBLICATION testpub3, testpub4, testpub5, testpub6, testpub7;
+DROP PUBLICATION testpub_dup1, testpub_dup2;
 
 ---------------------------------------------
 -- EXCEPT clause tests for partitioned tables
@@ -335,8 +349,15 @@ UPDATE testpub_parted2 SET a = 2;
 ALTER PUBLICATION testpub_forparted DROP TABLE testpub_parted;
 -- works again, because update is no longer replicated
 UPDATE testpub_parted2 SET a = 2;
+-- Mixing ONLY and non-ONLY mentions of the same partitioned table is silently
+-- tolerated because ONLY has no effect for partitioned tables anyway.
+CREATE PUBLICATION testpub_dup1 FOR TABLE testpub_parted, TABLE ONLY testpub_parted;
+\dRp+ testpub_dup1
+CREATE PUBLICATION testpub_dup2 FOR TABLE ONLY testpub_parted, TABLE testpub_parted;
+\dRp+ testpub_dup2
+
 DROP TABLE testpub_parted1, testpub_parted2;
-DROP PUBLICATION testpub_forparted, testpub_forparted1;
+DROP PUBLICATION testpub_forparted, testpub_forparted1, testpub_dup1, testpub_dup2;
 
 -- Tests for row filters
 CREATE TABLE testpub_rf_tbl1 (a integer, b text);