From af13ea0f0317573142b7b7aecf01b55e9739323f Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy Date: Wed, 16 Sep 2020 21:55:14 +0530 Subject: [PATCH v6] 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 | 39 +++++++++++++++++----- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c index 9ee70a2563..8ceba81d50 100644 --- a/src/backend/replication/logical/relation.c +++ b/src/backend/replication/logical/relation.c @@ -285,11 +285,11 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode) if (!entry->localrelvalid) { Oid relid; - int found; Bitmapset *idkey; TupleDesc desc; MemoryContext oldctx; int i; + Bitmapset *missingatts; /* Try to find and lock the relation by name. */ relid = RangeVarGetRelid(makeRangeVar(remoterel->nspname, @@ -317,7 +317,7 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode) entry->attrmap = make_attrmap(desc->natts); MemoryContextSwitchTo(oldctx); - found = 0; + missingatts = bms_add_range(NULL, 0, remoterel->natts - 1); for (i = 0; i < desc->natts; i++) { int attnum; @@ -334,16 +334,39 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode) entry->attrmap->attnums[i] = attnum; if (attnum >= 0) - found++; + missingatts = bms_del_member(missingatts, attnum); } - /* TODO, detail message with names of missing columns */ - if (found < remoterel->natts) + /* Report error with names of the missing localrel column(s). */ + if (!bms_is_empty(missingatts)) + { + StringInfoData missingattsbuf; + int missingattcnt = 0; + + initStringInfo(&missingattsbuf); + while ((i = bms_first_member(missingatts)) >= 0) + { + missingattcnt++; + if (missingattcnt == 1) + appendStringInfo(&missingattsbuf, _("\"%s\""), + remoterel->attnames[i]); + else + appendStringInfo(&missingattsbuf, _(", \"%s\""), + remoterel->attnames[i]); + } + + bms_free(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, + missingattsbuf.data))); + } /* * Check that replica identity matches. We allow for stricter replica -- 2.25.1