From 675db254ca6f367cc2d3ea1b6fcce57c57d5a654 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddy@enterprisedb.com>
Date: Tue, 8 Sep 2020 22:20:09 +0530
Subject: [PATCH v3] Detail message with names of missing columns in logical
 replication

In logical replication when a subscriber is missing some columns,
it currently emits an error message that says "some" columns are
missing(see logicalrep_rel_open()), but it doesn't specify what
the missing column names are. The comment there also says that
finding the missing column names is a todo item(/* TODO, detail
message with names of missing columns */).

This patch finds the missing columns on the subscriber relation
using the publisher relation columns and show them in the error
message which makes error to be more informative to the user.
---
 src/backend/replication/logical/relation.c | 70 ++++++++++++++++++++--
 1 file changed, 66 insertions(+), 4 deletions(-)

diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index a60c73d74d..5177b3f8cd 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -227,6 +227,54 @@ logicalrep_rel_att_by_name(LogicalRepRelation *remoterel, const char *attname)
 	return -1;
 }
 
+/*
+ * Finds the attributes that are missing in the local relation
+ * compared to remote relation.
+ * Returns the number of missing atts.
+ */
+static int
+logicalrep_find_missing_atts(AttrMap    *attrmap,
+							 LogicalRepRelation *remoterel,
+							 StringInfo missingatts)
+{
+	int			i;
+	int			missingattcnt = 0;
+	Bitmapset   *localattnums = NULL;
+
+	/* Add local relation attnums to a bitmapset. */
+	for (i = 0; i < attrmap->maplen; i++)
+	{
+		/*
+		 * Do not add attnums with values -1 to bitmapset
+		 * as it doesn't take negative values. Attnums value
+		 * is -1 for a dropped or a generated local attribute.
+		 */
+		if (attrmap->attnums[i] >= 0)
+			localattnums = bms_add_member(localattnums, attrmap->attnums[i]);
+	}
+
+	/* Find the remote attributes that are missing in the local relation. */
+	for (i = 0; i < remoterel->natts; i++)
+	{
+		if (!bms_is_member(i, localattnums))
+		{
+			if (missingatts->len == 0)
+				appendStringInfoString(missingatts,
+									   quote_identifier(remoterel->attnames[i]));
+			else if (missingatts->len > 0)
+			{
+				appendStringInfoChar(missingatts, ',');
+				appendStringInfo(missingatts, "%s",
+								 quote_identifier(remoterel->attnames[i]));
+			}
+			missingattcnt++;
+		}
+	}
+	bms_free(localattnums);
+
+	return missingattcnt;
+}
+
 /*
  * Open the local relation associated with the remote one.
  *
@@ -322,13 +370,27 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode)
 				found++;
 		}
 
-		/* TODO, detail message with names of missing columns */
+		/* Report error with names of the missing localrel column(s). */
 		if (found < remoterel->natts)
+		{
+			StringInfoData missingatts;
+			int			   missingattcnt;
+
+			initStringInfo(&missingatts);
+			missingattcnt = logicalrep_find_missing_atts(entry->attrmap,
+														 remoterel,
+														 &missingatts);
 			ereport(ERROR,
 					(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-					 errmsg("logical replication target relation \"%s.%s\" is missing "
-							"some replicated columns",
-							remoterel->nspname, remoterel->relname)));
+					 errmsg_plural("logical replication target relation \"%s.%s\" is missing "
+							"replicated column:%s",
+							"logical replication target relation \"%s.%s\" is missing "
+							"replicated columns:%s",
+							missingattcnt,
+							remoterel->nspname,
+							remoterel->relname,
+							missingatts.data)));
+		}
 
 		/*
 		 * Check that replica identity matches. We allow for stricter replica
-- 
2.25.1