From b8abd662b36aa588a7566fa7da3a2b1cd78d5e51 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Fri, 29 Apr 2022 17:41:34 +0200
Subject: [PATCH 1/2] Change some comments

---
 src/backend/replication/logical/relation.c  | 15 ++---
 src/backend/replication/logical/tablesync.c | 72 ++++++++++++---------
 2 files changed, 49 insertions(+), 38 deletions(-)

diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index 80fb561a9a..9307a6bf1d 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -119,14 +119,13 @@ logicalrep_relmap_init(void)
 }
 
 /*
- * Free the entry of a relation map cache.
+ * Clear the contents of a LogicalRepRelMapEntry, freeing any subsidiary
+ * allocated members.  The entry itself is not freed.
  */
 static void
-logicalrep_relmap_free_entry(LogicalRepRelMapEntry *entry)
+logicalrep_relmap_clear_entry(LogicalRepRelMapEntry *entry)
 {
-	LogicalRepRelation *remoterel;
-
-	remoterel = &entry->remoterel;
+	LogicalRepRelation *remoterel = &entry->remoterel;
 
 	pfree(remoterel->nspname);
 	pfree(remoterel->relname);
@@ -165,13 +164,13 @@ logicalrep_relmap_update(LogicalRepRelation *remoterel)
 		logicalrep_relmap_init();
 
 	/*
-	 * HASH_ENTER returns the existing entry if present or creates a new one.
+	 * Find an hashtable entry for this rel, or create a new one.  If there
+	 * was one, clear its contents so we can fill it fresh.
 	 */
 	entry = hash_search(LogicalRepRelMap, (void *) &remoterel->remoteid,
 						HASH_ENTER, &found);
-
 	if (found)
-		logicalrep_relmap_free_entry(entry);
+		logicalrep_relmap_clear_entry(entry);
 
 	memset(entry, 0, sizeof(LogicalRepRelMapEntry));
 
diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c
index fd547e16f4..82ee406eb4 100644
--- a/src/backend/replication/logical/tablesync.c
+++ b/src/backend/replication/logical/tablesync.c
@@ -1100,15 +1100,14 @@ copy_table(Relation rel)
 	}
 	else
 	{
+		List	   *quals;
+		ListCell   *lc;
+
 		/*
 		 * For non-tables and tables with row filters, we need to do COPY
 		 * (SELECT ...), but we can't just do SELECT * because we need to not
 		 * copy generated columns. For tables with any row filters, build a
 		 * SELECT query with OR'ed row filters for COPY.
-		 *
-		 * FIXME can be simplified if all subscriptions have the same column
-		 * list (or no column list), in which case we don't need the CASE
-		 * expressions at all.
 		 */
 		appendStringInfoString(&cmd, "COPY (SELECT ");
 		for (int i = 0; i < lrel.natts; i++)
@@ -1146,7 +1145,17 @@ copy_table(Relation rel)
 					appendStringInfo(&qual, " OR %s", strVal(pubinfo->rowfilter));
 			}
 
-			if (no_filter)
+			/*
+			 * If there's no row filter (which includes the case of there
+			 * being several publications of which at least one does not have
+			 * a row filter), or there is a single publication, then we can
+			 * emit the column name itself (the latter is possible because the
+			 * WHERE clause will take care of omitting these rows; this works
+			 * only with a single publication.)  If there are several
+			 * publications, then emit a CASE expression that only prints the
+			 * column if the row meets the row filter of any publications.
+			 */
+			if (no_filter || list_length(pubinfos) == 1)
 				appendStringInfoString(&cmd, quote_identifier(lrel.attnames[i]));
 			else
 				appendStringInfo(&cmd, "(CASE WHEN (%s) THEN %s ELSE NULL END) AS %s",
@@ -1169,38 +1178,41 @@ copy_table(Relation rel)
 
 		appendStringInfoString(&cmd, quote_qualified_identifier(lrel.nspname, lrel.relname));
 
+		/*
+		 * Now, based on the row filters, build a WHERE clause for the query.
+		 * If there's at least one publication with no row filter, then the
+		 * overall WHERE reduces to empty.
+		 */
+
+		quals = NIL;
+		foreach (lc, pubinfos)
 		{
-			List *qual = NIL;
-			ListCell *lc;
+			PublicationInfo *pubinfo = (PublicationInfo *) lfirst(lc);
 
-			foreach (lc, pubinfos)
+			if (pubinfo->rowfilter == NULL)
 			{
-				PublicationInfo *pubinfo = (PublicationInfo *) lfirst(lc);
-
-				if (pubinfo->rowfilter == NULL)
-				{
-					qual = NIL;
-					break;
-				}
-
-				qual = lappend(qual, pubinfo->rowfilter);
+				quals = NIL;
+				break;
 			}
 
-		/* list of OR'ed filters */
-		if (qual != NIL)
-		{
-			ListCell   *lc;
-			char	   *q = strVal(linitial(qual));
-
-			appendStringInfo(&cmd, " WHERE %s", q);
-			for_each_from(lc, qual, 1)
-			{
-				q = strVal(lfirst(lc));
-				appendStringInfo(&cmd, " OR %s", q);
-			}
-			list_free_deep(qual);
+			quals = lappend(quals, pubinfo->rowfilter);
 		}
 
+		/*
+		 * If we do have filters, write a WHERE clause that ORs them all
+		 * together.
+		 */
+		if (quals != NIL)
+		{
+			char	   *q = strVal(linitial(quals));
+
+			appendStringInfo(&cmd, " WHERE (%s)", q);
+			for_each_from(lc, quals, 1)
+			{
+				q = strVal(lfirst(lc));
+				appendStringInfo(&cmd, " OR (%s)", q);
+			}
+			list_free_deep(quals);
 		}
 
 		appendStringInfoString(&cmd, ") TO STDOUT");
-- 
2.30.2

