From af38de251ac5fb321f19273ec079eac3f08f4f08 Mon Sep 17 00:00:00 2001
From: "houzj.fnst" <houzj.fnst@fujitsu.com>
Date: Fri, 15 Oct 2021 18:23:30 +0800
Subject: [PATCH] fix double publish

if publish_via_partition_root is true, then the child table's data will be
copied twice if adding both child and parent table to the publication. The
reason is that the subscriber will fetch the table list from publisher's
pg_publication_tables view to do the table synchronization. But the view always
show both child and parent table which cause the extra synchronization
for the child table.

Fix it by making pg_publication_tables only show parent table if both parent
and child exists in the publication.

---
 src/backend/catalog/pg_publication.c      | 51 +++++++++++++++++++++--
 src/backend/commands/publicationcmds.c    |  8 ++--
 src/include/catalog/pg_publication.h      |  3 +-
 src/test/regress/expected/publication.out |  9 ++++
 src/test/regress/sql/publication.sql      |  4 ++
 5 files changed, 67 insertions(+), 8 deletions(-)

diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c
index 9cd0c82f93..8f039599ee 100644
--- a/src/backend/catalog/pg_publication.c
+++ b/src/backend/catalog/pg_publication.c
@@ -291,11 +291,18 @@ GetRelationPublications(Oid relid)
  *
  * This should only be used FOR TABLE publications, the FOR ALL TABLES
  * should use GetAllTablesPublicationRelations().
+ *
+ * If pubviaroot is true, we must exclude partitions in favor of including the
+ * root partitioned tables.
  */
 List *
-GetPublicationRelations(Oid pubid, PublicationPartOpt pub_partopt)
+GetPublicationRelations(Oid pubid, PublicationPartOpt pub_partopt,
+						bool pubviaroot)
 {
 	List	   *result;
+	List	   *relids;
+	ListCell   *lc;
+	ListCell   *lc2;
 	Relation	pubrelsrel;
 	ScanKeyData scankey;
 	SysScanDesc scan;
@@ -312,19 +319,54 @@ GetPublicationRelations(Oid pubid, PublicationPartOpt pub_partopt)
 	scan = systable_beginscan(pubrelsrel, PublicationRelPrrelidPrpubidIndexId,
 							  true, NULL, 1, &scankey);
 
-	result = NIL;
+	result = relids = NIL;
+
 	while (HeapTupleIsValid(tup = systable_getnext(scan)))
 	{
 		Form_pg_publication_rel pubrel;
 
 		pubrel = (Form_pg_publication_rel) GETSTRUCT(tup);
-		result = GetPubPartitionOptionRelations(result, pub_partopt,
+		relids = GetPubPartitionOptionRelations(relids, pub_partopt,
 												pubrel->prrelid);
 	}
 
 	systable_endscan(scan);
 	table_close(pubrelsrel, AccessShareLock);
 
+	if (!pubviaroot)
+		return relids;
+
+	foreach(lc, relids)
+	{
+		bool		skip = false;
+		List	   *ancestors = NIL;
+		Oid			relid = lfirst_oid(lc);
+
+		/*
+		 * Filter out the partitions whose parent tables was also specified in
+		 * the publication.
+		 */
+		if (get_rel_relispartition(relid))
+			ancestors = get_partition_ancestors(relid);
+
+		foreach(lc2, ancestors)
+		{
+			/*
+			 * Check if the parent table exists in the published table list or
+			 * the schema of the parent table was published in this
+			 * publication.
+			 */
+			if (list_member_oid(relids, lfirst_oid(lc2)))
+			{
+				skip = true;
+				break;
+			}
+		}
+
+		if (!skip)
+			result = lappend_oid(result, relid);
+	}
+
 	return result;
 }
 
@@ -560,7 +602,8 @@ pg_get_publication_tables(PG_FUNCTION_ARGS)
 			tables = GetPublicationRelations(publication->oid,
 											 publication->pubviaroot ?
 											 PUBLICATION_PART_ROOT :
-											 PUBLICATION_PART_LEAF);
+											 PUBLICATION_PART_LEAF,
+											 publication->pubviaroot);
 		funcctx->user_fctx = (void *) tables;
 
 		MemoryContextSwitchTo(oldcontext);
diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c
index 9c7f91611d..836b6cf421 100644
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -324,7 +324,8 @@ AlterPublicationOptions(ParseState *pstate, AlterPublicationStmt *stmt,
 		 * trees, not just those explicitly mentioned in the publication.
 		 */
 		List	   *relids = GetPublicationRelations(pubform->oid,
-													 PUBLICATION_PART_ALL);
+													 PUBLICATION_PART_ALL,
+													 false);
 
 		InvalidatePublicationRels(relids);
 	}
@@ -387,7 +388,8 @@ AlterPublicationTables(AlterPublicationStmt *stmt, Relation rel,
 	else						/* DEFELEM_SET */
 	{
 		List	   *oldrelids = GetPublicationRelations(pubid,
-														PUBLICATION_PART_ROOT);
+														PUBLICATION_PART_ROOT,
+														false);
 		List	   *delrels = NIL;
 		ListCell   *oldlc;
 
@@ -538,7 +540,7 @@ RemovePublicationById(Oid pubid)
 	if (!HeapTupleIsValid(tup))
 		elog(ERROR, "cache lookup failed for publication %u", pubid);
 
-	pubform = (Form_pg_publication)GETSTRUCT(tup);
+	pubform = (Form_pg_publication) GETSTRUCT(tup);
 
 	/* Invalidate relcache so that publication info is rebuilt. */
 	if (pubform->puballtables)
diff --git a/src/include/catalog/pg_publication.h b/src/include/catalog/pg_publication.h
index 82f2536c65..721233b226 100644
--- a/src/include/catalog/pg_publication.h
+++ b/src/include/catalog/pg_publication.h
@@ -108,7 +108,8 @@ typedef enum PublicationPartOpt
 	PUBLICATION_PART_ALL,
 } PublicationPartOpt;
 
-extern List *GetPublicationRelations(Oid pubid, PublicationPartOpt pub_partopt);
+extern List *GetPublicationRelations(Oid pubid, PublicationPartOpt pub_partopt,
+									 bool pubviaroot);
 extern List *GetAllTablesPublications(void);
 extern List *GetAllTablesPublicationRelations(bool pubviaroot);
 extern List *GetPubPartitionOptionRelations(List *result,
diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out
index 82bce9be09..5f5445a985 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -165,6 +165,15 @@ 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;
+-- add both child and parent table to the publication
+ALTER PUBLICATION testpub_forparted ADD TABLE testpub_parted, testpub_parted2;
+-- only show parent table when publish_via_partition_root is set
+select * from pg_publication_tables;
+      pubname      | schemaname |   tablename    
+-------------------+------------+----------------
+ testpub_forparted | public     | testpub_parted
+(1 row)
+
 DROP TABLE testpub_parted1, testpub_parted2;
 DROP PUBLICATION testpub_forparted, testpub_forparted1;
 -- Test cache invalidation FOR ALL TABLES publication
diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql
index e5745d575b..d3f2b2fa11 100644
--- a/src/test/regress/sql/publication.sql
+++ b/src/test/regress/sql/publication.sql
@@ -97,6 +97,10 @@ 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;
+-- add both child and parent table to the publication
+ALTER PUBLICATION testpub_forparted ADD TABLE testpub_parted, testpub_parted2;
+-- only show parent table when publish_via_partition_root is set
+select * from pg_publication_tables;
 DROP TABLE testpub_parted1, testpub_parted2;
 DROP PUBLICATION testpub_forparted, testpub_forparted1;
 
-- 
2.18.4

