Re: logical replication - negative bitmapset member not allowed

On Thu, 10 Oct 2019 15:15:46 +0200
Jehan-Guillaume de Rorthais <jgdr(at)dalibo(dot)com> wrote:

[...]
> Here is a script to reproduce it under version 10, 11 and 12:

I investigated on this bug while coming back from pgconf.eu. Bellow what I found
so far.

The message "negative bitmapset member not allowed" comes from
logicalrep_rel_open().

Every field that are unknown, dropped or generated are mapped to remote attnum
-1. See backend/replication/logical/relation.c:

if (attr->attisdropped || attr->attgenerated)
{
entry->attrmap[i] = -1;
continue;
}

attnum = logicalrep_rel_att_by_name(remoterel, NameStr(attr->attname));

Note that logicalrep_rel_att_by_name returns -1 on unknown fields.

Later in the same function, we check if fields belonging to some PK or unique
index appears in remote keys as well:

while ((i = bms_next_member(idkey, i)) >= 0)
{
[...]
if (!bms_is_member(entry->attrmap[attnum], remoterel->attkeys))
{
entry->updatable = false;
break;
}
}

However, before checking if the local attribute belong to the remote keys,
it should check if it actually mapped to a remote one. In other words, I
suppose we should check entry->attrmap[attnum] > 0 before calling
bms_is_member().

The trivial patch would be:

- if (!bms_is_member(entry->attrmap[attnum], remoterel->attkeys))
+ if (entry->attrmap[attnum] < 0 ||
+ !bms_is_member(entry->attrmap[attnum], remoterel->attkeys))
{
entry->updatable = false;
break;
}

I tested with the attached scenario and it sound to work correctly.

Note that while trying to fix this bug, I found a segment fault while compiling
with asserts. You might want to review/test without --enable-cassert. I will
report in another thread as this seems not related to this bug or fix.