Thanks for the comments, v2 patch is attached.
>
> On Tue, Sep 8, 2020 at 6:50 AM Kyotaro Horiguchi
> <horikyota....@gmail.com> wrote:
> >
> > +1 for objective. However, that can be done simpler way that doesn't
> > need additional loops by using bitmapset to hold missing remote
> > attribute numbers. This also make the variable "found" useless.
> >
>
> Thanks. I will look into it and post a v2 patch soon.
>
Changed.
> >
> > FWIW, I would prefer that the message be like
> >
> > logical replication target relation "public.test_1" is missing
> > replicated columns: "a1", "c1"
> >
>
> This looks fine, I will change that.
>
Changed. Now the error looks like as shown below:
ERROR: logical replication target relation "public.test_tbl1" is
missing replicated columns:"d1"
ERROR: logical replication target relation "public.test_tbl1" is
missing replicated columns:"c1","d1"
ERROR: logical replication target relation "public.test_tbl1" is
missing replicated columns:"a1","c1","d1"
ERROR: logical replication target relation "public.test_tbl1" is
missing replicated columns:"a1","b1","c1","d1"
ERROR: logical replication target relation "public.test_tbl1" is
missing replicated columns:"a1","b1","c1","d1","e1"
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
From 9fbda09a015895a447bdce6683e8138350b133bd Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddy@enterprisedb.com>
Date: Tue, 8 Sep 2020 02:45:04 -0700
Subject: [PATCH v2] 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 | 60 ++++++++++++++++++++--
1 file changed, 57 insertions(+), 3 deletions(-)
diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index a60c73d74d..2ecb4766a4 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -227,6 +227,53 @@ 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.
+ */
+static void
+logicalrep_find_missing_atts(AttrMap *attrmap,
+ LogicalRepRelation *remoterel,
+ StringInfo missingatts)
+{
+ int i;
+ 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)
+ {
+ appendStringInfoChar(missingatts, '"');
+ appendStringInfoString(missingatts, remoterel->attnames[i]);
+ }
+ else if (missingatts->len > 0)
+ {
+ appendStringInfoChar(missingatts, ',');
+ appendStringInfoChar(missingatts, '"');
+ appendStringInfo(missingatts, "%s", remoterel->attnames[i]);
+ }
+
+ appendStringInfoChar(missingatts, '"');
+ }
+ }
+ bms_free(localattnums);
+}
+
/*
* Open the local relation associated with the remote one.
*
@@ -322,13 +369,20 @@ 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;
+
+ initStringInfo(&missingatts);
+ 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)));
+ "replicated columns:%s",
+ remoterel->nspname, remoterel->relname,
+ missingatts.data)));
+ }
/*
* Check that replica identity matches. We allow for stricter replica
--
2.25.1
