Fix construction of updated-columns bitmap in logical replication.

Commit b9c130a1f failed to apply the publisher-to-subscriber column
mapping while checking which columns were updated.  Perhaps less
significantly, it didn't exclude dropped columns either.  This could
result in an incorrect updated-columns bitmap and thus wrong decisions
about whether to fire column-specific triggers on the subscriber while
applying updates.  In HEAD (since commit 9de77b545), it could also
result in accesses off the end of the colstatus array, as detected by
buildfarm member skink.  Fix the logic, and adjust 003_constraints.pl
so that the problem is exposed in unpatched code.

In HEAD, also add some assertions to check that we don't access off
the ends of these newly variable-sized arrays.

Back-patch to v10, as b9c130a1f was.

Discussion: https://postgr.es/m/CAH2-Wz=79hKQ4++c5A060RYbjTHgiYTHz=fw6mptCtgghH2gJA@mail.gmail.com

diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 7e3f3f0b0c05483809cb372d816e023b608b92b9..d0064fb7b55b4304ba7d55277492567f8d6be2ed 100644 (file)
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -745,9 +745,16 @@ apply_handle_update(StringInfo s)
    target_rte = list_nth(estate->es_range_table, 0);
    for (i = 0; i < remoteslot->tts_tupleDescriptor->natts; i++)
    {
-       if (newtup.changed[i])
-           target_rte->updatedCols = bms_add_member(target_rte->updatedCols,
-                                                    i + 1 - FirstLowInvalidHeapAttributeNumber);
+       Form_pg_attribute att = TupleDescAttr(remoteslot->tts_tupleDescriptor, i);
+       int         remoteattnum = rel->attrmap[i];
+
+       if (!att->attisdropped && remoteattnum >= 0)
+       {
+           if (newtup.changed[remoteattnum])
+               target_rte->updatedCols =
+                   bms_add_member(target_rte->updatedCols,
+                                  i + 1 - FirstLowInvalidHeapAttributeNumber);
+       }
    }
 
    PushActiveSnapshot(GetTransactionSnapshot());

diff --git a/src/test/subscription/t/003_constraints.pl b/src/test/subscription/t/003_constraints.pl
index 65528edf537fcdeba6bb21299b919689f6407876..db29bfcaec383f47a0c290a4e68e5c1eb55f399b 100644 (file)
--- a/src/test/subscription/t/003_constraints.pl
+++ b/src/test/subscription/t/003_constraints.pl
@@ -19,14 +19,14 @@ $node_subscriber->start;
 $node_publisher->safe_psql('postgres',
    "CREATE TABLE tab_fk (bid int PRIMARY KEY);");
 $node_publisher->safe_psql('postgres',
-"CREATE TABLE tab_fk_ref (id int PRIMARY KEY, bid int REFERENCES tab_fk (bid));"
+"CREATE TABLE tab_fk_ref (id int PRIMARY KEY, junk text, bid int REFERENCES tab_fk (bid));"
 );
 
-# Setup structure on subscriber
+# Setup structure on subscriber; column order intentionally different
 $node_subscriber->safe_psql('postgres',
    "CREATE TABLE tab_fk (bid int PRIMARY KEY);");
 $node_subscriber->safe_psql('postgres',
-"CREATE TABLE tab_fk_ref (id int PRIMARY KEY, bid int REFERENCES tab_fk (bid));"
+"CREATE TABLE tab_fk_ref (id int PRIMARY KEY, bid int REFERENCES tab_fk (bid), junk text);"
 );
 
 # Setup logical replication
@@ -47,8 +47,10 @@ $node_publisher->poll_query_until('postgres', $caughtup_query)
 
 $node_publisher->safe_psql('postgres',
    "INSERT INTO tab_fk (bid) VALUES (1);");
+# "junk" value is meant to be large enough to force out-of-line storage
 $node_publisher->safe_psql('postgres',
-   "INSERT INTO tab_fk_ref (id, bid) VALUES (1, 1);");
+   "INSERT INTO tab_fk_ref (id, bid, junk) VALUES (1, 1, repeat(pi()::text,20000));"
+);
 
 $node_publisher->poll_query_until('postgres', $caughtup_query)
   or die "Timed out while waiting for subscriber to catch up";
@@ -136,7 +138,8 @@ $node_publisher->poll_query_until('postgres', $caughtup_query)
 
 $result = $node_subscriber->safe_psql('postgres',
    "SELECT count(*), min(id), max(id) FROM tab_fk_ref;");
-is($result, qq(2|1|2), 'check column trigger applied on even for other column');
+is($result, qq(2|1|2),
+   'check column trigger applied even on update for other column');
 
 $node_subscriber->stop('fast');
 $node_publisher->stop('fast');