From 0e650ef40424ffd36dfbb18d92635d8c5b91c377 Mon Sep 17 00:00:00 2001
From: "houzj.fnst" <houzj.fnst@fujitsu.com>
Date: Thu, 11 Nov 2021 17:31:05 +0800
Subject: [PATCH] Invalidate relcache when setting REPLICA IDENTITY

When changing REPLICA IDENTITY INDEX to another one, the target table's
relcache would not be invalidated which could cause unexpected behaviour when
replicating UPDATE change.

So, Invalidate the relcache in this case to make sure the the bitmap of index
attribute is rebuilt

Author: Tang Haiying, Hou Zhijie
Reviewed-by: Amit Kapila

---
 src/backend/commands/tablecmds.c    | 24 +++++++---
 src/test/subscription/t/100_bugs.pl | 69 ++++++++++++++++++++++++++++-
 2 files changed, 85 insertions(+), 8 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 857cc5ce6e..8306fd1eb8 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -15412,6 +15412,7 @@ relation_mark_replica_identity(Relation rel, char ri_type, Oid indexOid,
 	Form_pg_index pg_index_form;
 
 	ListCell   *index;
+	bool		need_rel_inval = false;
 
 	/*
 	 * Check whether relreplident has changed, and update it if so.
@@ -15488,10 +15489,19 @@ relation_mark_replica_identity(Relation rel, char ri_type, Oid indexOid,
 			CatalogTupleUpdate(pg_index, &pg_index_tuple->t_self, pg_index_tuple);
 			InvokeObjectPostAlterHookArg(IndexRelationId, thisIndexOid, 0,
 										 InvalidOid, is_internal);
+			need_rel_inval = true;
 		}
 		heap_freetuple(pg_index_tuple);
 	}
 
+	/*
+	 * Invalidate the relcache for the table, so that after we commit all
+	 * sessions will refresh the table's replica identity index before
+	 * attempting any update on the table.
+	 */
+	if (need_rel_inval)
+		CacheInvalidateRelcache(rel);
+
 	table_close(pg_index, RowExclusiveLock);
 }
 
@@ -17525,12 +17535,12 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd,
 	/*
 	 * If the partition we just attached is partitioned itself, invalidate
 	 * relcache for all descendent partitions too to ensure that their
-	 * rd_partcheck expression trees are rebuilt; partitions already locked
-	 * at the beginning of this function.
+	 * rd_partcheck expression trees are rebuilt; partitions already locked at
+	 * the beginning of this function.
 	 */
 	if (attachrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
 	{
-		ListCell *l;
+		ListCell   *l;
 
 		foreach(l, attachrel_children)
 		{
@@ -18232,13 +18242,13 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
 	/*
 	 * If the partition we just detached is partitioned itself, invalidate
 	 * relcache for all descendent partitions too to ensure that their
-	 * rd_partcheck expression trees are rebuilt; must lock partitions
-	 * before doing so, using the same lockmode as what partRel has been
-	 * locked with by the caller.
+	 * rd_partcheck expression trees are rebuilt; must lock partitions before
+	 * doing so, using the same lockmode as what partRel has been locked with
+	 * by the caller.
 	 */
 	if (partRel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
 	{
-		List   *children;
+		List	   *children;
 
 		children = find_all_inheritors(RelationGetRelid(partRel),
 									   AccessExclusiveLock, NULL);
diff --git a/src/test/subscription/t/100_bugs.pl b/src/test/subscription/t/100_bugs.pl
index 34a60fd9ab..a62b16959e 100644
--- a/src/test/subscription/t/100_bugs.pl
+++ b/src/test/subscription/t/100_bugs.pl
@@ -6,7 +6,7 @@ use strict;
 use warnings;
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
-use Test::More tests => 5;
+use Test::More tests => 7;
 
 # Bug #15114
 
@@ -224,3 +224,70 @@ $node_sub->safe_psql('postgres', "DROP TABLE tab1");
 $node_pub->stop('fast');
 $node_pub_sub->stop('fast');
 $node_sub->stop('fast');
+
+# Test the replication of the table after changing REPLICA IDENTITY INDEX
+
+# The bug was that when changing REPLICA IDENTITY INDEX to another one, the
+# target table's relcache would not be invalidated. This could cause unexpected
+# behaviour when replicating UPDATE change.
+$node_publisher = PostgreSQL::Test::Cluster->new('publisher3');
+$node_publisher->init(allows_streaming => 'logical');
+$node_publisher->start;
+
+$node_subscriber = PostgreSQL::Test::Cluster->new('subscriber3');
+$node_subscriber->init(allows_streaming => 'logical');
+$node_subscriber->start;
+
+$node_publisher->safe_psql('postgres',
+	"CREATE TABLE tab_replidentity_index(a int not null, b int not null)");
+$node_publisher->safe_psql('postgres',
+	"CREATE UNIQUE INDEX idx_replidentity_index_a ON tab_replidentity_index(a)"
+);
+$node_publisher->safe_psql('postgres',
+	"CREATE UNIQUE INDEX idx_replidentity_index_b ON tab_replidentity_index(b)"
+);
+
+# use index idx_replidentity_index_a as REPLICA IDENTITY in publisher.
+$node_publisher->safe_psql('postgres',
+	"ALTER TABLE tab_replidentity_index REPLICA IDENTITY USING INDEX idx_replidentity_index_a;"
+);
+$node_publisher->safe_psql('postgres',
+	"INSERT INTO tab_replidentity_index VALUES(1, 1)");
+
+$node_subscriber->safe_psql('postgres',
+	"CREATE TABLE tab_replidentity_index(a int not null, b int not null)");
+$node_subscriber->safe_psql('postgres',
+	"CREATE UNIQUE INDEX idx_replidentity_index_b ON tab_replidentity_index(b)"
+);
+
+$publisher_connstr = $node_publisher->connstr . ' dbname=postgres';
+$node_publisher->safe_psql('postgres',
+	"CREATE PUBLICATION tap_pub FOR TABLE tab_replidentity_index");
+$node_subscriber->safe_psql('postgres',
+	"CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr' PUBLICATION tap_pub"
+);
+
+$node_publisher->wait_for_catchup('tap_sub');
+
+is($node_subscriber->safe_psql('postgres', "SELECT * FROM tab_replidentity_index"),
+	qq(1|1),
+	"check initial data on subscriber");
+
+# use index idx_replidentity_index_a as REPLICA IDENTITY in subcriber.
+$node_subscriber->safe_psql('postgres', qq[
+	ALTER TABLE tab_replidentity_index REPLICA IDENTITY USING INDEX idx_replidentity_index_b;
+]);
+
+# Set publisher's REPLICA IDENTITY to idx_replidentity_index_b, then UPDATE the table.
+$node_publisher->safe_psql('postgres', qq[
+	ALTER TABLE tab_replidentity_index REPLICA IDENTITY USING INDEX idx_replidentity_index_b;
+	UPDATE tab_replidentity_index SET a = -a;
+]);
+
+$node_publisher->wait_for_catchup('tap_sub');
+is( $node_subscriber->safe_psql('postgres', "SELECT * FROM tab_replidentity_index"),
+	qq(-1|1),
+	"update works with REPLICA IDENTITY");
+
+$node_publisher->stop('fast');
+$node_subscriber->stop('fast');
-- 
2.18.4

