Re: logical replication - negative bitmapset member not allowed

On Thu, 10 Oct 2019 15:15:46 +0200
Jehan-Guillaume de Rorthais <jgdr@dalibo.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.