doc: improve the restriction description of using indexes on REPLICA IDENTITY FULL table.

diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index c5de2040f7..1acf75be6f 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -134,12 +134,12 @@
    to replica identity <literal>FULL</literal>, which means the entire row becomes
    the key.  When replica identity <literal>FULL</literal> is specified,
    indexes can be used on the subscriber side for searching the rows.  Candidate
-   indexes must be btree, non-partial, and have at least one column reference
-   (i.e. cannot consist of only expressions).  These restrictions
-   on the non-unique index properties adhere to some of the restrictions that
-   are enforced for primary keys.  If there are no such suitable indexes,
-   the search on the subscriber side can be very inefficient, therefore
-   replica identity <literal>FULL</literal> should only be used as a
+   indexes must be btree, non-partial, and have at least one column reference to
+   the published table column at the leftmost column (i.e. cannot consist of only
+   expressions).  These restrictions on the non-unique index properties adhere to
+   some of the restrictions that are enforced for primary keys.  If there are no
+   such suitable indexes, the search on the subscriber side can be very inefficient,
+   therefore replica identity <literal>FULL</literal> should only be used as a
    fallback if no other solution is possible.  If a replica identity other
    than <literal>FULL</literal> is set on the publisher side, a replica identity
    comprising the same or fewer columns must also be set on the subscriber
diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c
index 9dd7168461..6f2a465133 100644
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -47,9 +47,9 @@ static bool tuples_equal(TupleTableSlot *slot1, TupleTableSlot *slot2,
  *
  * Returns how many columns to use for the index scan.
  *
- * This is not generic routine, it expects the idxrel to be a btree, non-partial
- * and have at least one column reference (i.e. cannot consist of only
- * expressions).
+ * This is not generic routine, idxrel must be PK, RI, or an index that can be
+ * used for REPLICA IDENTITY FULL table (see FindUsableIndexForReplicaIdentityFull()
+ * for details).
  *
  * By definition, replication identity of a rel meets all limitations associated
  * with that. Note that any other index could also meet these limitations.
diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index 57ad22b48a..ce66149b5e 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -780,8 +780,9 @@ RemoteRelContainsLeftMostColumnOnIdx(IndexInfo *indexInfo, AttrMap *attrmap)
 /*
  * Returns the oid of an index that can be used by the apply worker to scan
  * the relation. The index must be btree, non-partial, and have at least
- * one column reference (i.e. cannot consist of only expressions). These
- * limitations help to keep the index scan similar to PK/RI index scans.
+ * one column reference to the remote relation's column at the leftmost column
+ * (i.e. cannot consist of only expressions). These limitations help to keep the
+ * index scan similar to PK/RI index scans.
  *
  * Note that the limitations of index scans for replica identity full only
  * adheres to a subset of the limitations of PK/RI. For example, we support

From 2ff55c318971d34c68fb8a28b9acb97ab99bbf71 Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Fri, 7 Jul 2023 16:49:39 +0900
Subject: [PATCH v3] Doc: clarify the conditions of usable indexes for REPLICA
 IDENTITY FULL tables.

Commit 89e46da5e allowed REPLICA IDENTIFY FULL tables to use an index
on the subscriber during apply of update/delete.

The source code comments are also updated accordingly.

Reviewed-by: Amit Kapila, Peter Smith
Discussion: https://postgr.es/m/CAD21AoDJjffEvUFKXT27Q5U8-UU9JHv4rrJ9Ke8Zkc5UPWHLvA@mail.gmail.com
Backpatch-through: 16
---
 doc/src/sgml/logical-replication.sgml      | 8 ++++----
 src/backend/executor/execReplication.c     | 6 +++---
 src/backend/replication/logical/relation.c | 7 ++++---
 3 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index c5de2040f7..92c3043303 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -134,10 +134,10 @@
    to replica identity <literal>FULL</literal>, which means the entire row becomes
    the key.  When replica identity <literal>FULL</literal> is specified,
    indexes can be used on the subscriber side for searching the rows.  Candidate
-   indexes must be btree, non-partial, and have at least one column reference
-   (i.e. cannot consist of only expressions).  These restrictions
-   on the non-unique index properties adhere to some of the restrictions that
-   are enforced for primary keys.  If there are no such suitable indexes,
+   indexes must be btree, non-partial, and the leftmost index column must reference
+   a published table column (i.e. cannot consist of only expressions).  These
+   restrictions on the non-unique index properties adhere to some of the restrictions
+   that are enforced for primary keys.  If there are no such suitable indexes,
    the search on the subscriber side can be very inefficient, therefore
    replica identity <literal>FULL</literal> should only be used as a
    fallback if no other solution is possible.  If a replica identity other
diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c
index 9dd7168461..6f2a465133 100644
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -47,9 +47,9 @@ static bool tuples_equal(TupleTableSlot *slot1, TupleTableSlot *slot2,
  *
  * Returns how many columns to use for the index scan.
  *
- * This is not generic routine, it expects the idxrel to be a btree, non-partial
- * and have at least one column reference (i.e. cannot consist of only
- * expressions).
+ * This is not generic routine, idxrel must be PK, RI, or an index that can be
+ * used for REPLICA IDENTITY FULL table (see FindUsableIndexForReplicaIdentityFull()
+ * for details).
  *
  * By definition, replication identity of a rel meets all limitations associated
  * with that. Note that any other index could also meet these limitations.
diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index 57ad22b48a..8bc002cdfc 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -779,9 +779,10 @@ RemoteRelContainsLeftMostColumnOnIdx(IndexInfo *indexInfo, AttrMap *attrmap)
 
 /*
  * Returns the oid of an index that can be used by the apply worker to scan
- * the relation. The index must be btree, non-partial, and have at least
- * one column reference (i.e. cannot consist of only expressions). These
- * limitations help to keep the index scan similar to PK/RI index scans.
+ * the relation. The index must be btree, non-partial, and the leftmost column
+ * must reference a published table column (i.e. cannot consist of only
+ * expressions). These limitations help to keep the index scan similar to
+ * PK/RI index scans.
  *
  * Note that the limitations of index scans for replica identity full only
  * adheres to a subset of the limitations of PK/RI. For example, we support
-- 
2.31.1

From 5b55b9850b633c1b9767d3a95a49d0dfd98fecad Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Fri, 7 Jul 2023 16:49:39 +0900
Subject: [PATCH v4] Doc: clarify the conditions of usable indexes for REPLICA
 IDENTITY FULL tables.

Commit 89e46da5e allowed REPLICA IDENTIFY FULL tables to use an index
on the subscriber during apply of update/delete.

The source code comments are also updated accordingly.

Reviewed-by: Amit Kapila, Peter Smith
Discussion: https://postgr.es/m/CAD21AoDJjffEvUFKXT27Q5U8-UU9JHv4rrJ9Ke8Zkc5UPWHLvA@mail.gmail.com
Backpatch-through: 16
---
 doc/src/sgml/logical-replication.sgml      | 12 ++++++------
 src/backend/executor/execReplication.c     |  6 +++---
 src/backend/replication/logical/relation.c | 11 ++++-------
 3 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index c5de2040f7..e0996b84ea 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -134,12 +134,12 @@
    to replica identity <literal>FULL</literal>, which means the entire row becomes
    the key.  When replica identity <literal>FULL</literal> is specified,
    indexes can be used on the subscriber side for searching the rows.  Candidate
-   indexes must be btree, non-partial, and have at least one column reference
-   (i.e. cannot consist of only expressions).  These restrictions
-   on the non-unique index properties adhere to some of the restrictions that
-   are enforced for primary keys.  If there are no such suitable indexes,
-   the search on the subscriber side can be very inefficient, therefore
-   replica identity <literal>FULL</literal> should only be used as a
+   indexes must be btree, non-partial, and the leftmost index field must be a
+   column (not an expression) that reference a published table column.  These
+   restrictions on the non-unique index properties adhere to some of the
+   restrictions that are enforced for primary keys.  If there are no such
+   suitable indexes, the search on the subscriber side can be very inefficient,
+   therefore replica identity <literal>FULL</literal> should only be used as a
    fallback if no other solution is possible.  If a replica identity other
    than <literal>FULL</literal> is set on the publisher side, a replica identity
    comprising the same or fewer columns must also be set on the subscriber
diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c
index 9dd7168461..6f2a465133 100644
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -47,9 +47,9 @@ static bool tuples_equal(TupleTableSlot *slot1, TupleTableSlot *slot2,
  *
  * Returns how many columns to use for the index scan.
  *
- * This is not generic routine, it expects the idxrel to be a btree, non-partial
- * and have at least one column reference (i.e. cannot consist of only
- * expressions).
+ * This is not generic routine, idxrel must be PK, RI, or an index that can be
+ * used for REPLICA IDENTITY FULL table (see FindUsableIndexForReplicaIdentityFull()
+ * for details).
  *
  * By definition, replication identity of a rel meets all limitations associated
  * with that. Note that any other index could also meet these limitations.
diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index 57ad22b48a..fe940335a3 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -779,9 +779,10 @@ RemoteRelContainsLeftMostColumnOnIdx(IndexInfo *indexInfo, AttrMap *attrmap)
 
 /*
  * Returns the oid of an index that can be used by the apply worker to scan
- * the relation. The index must be btree, non-partial, and have at least
- * one column reference (i.e. cannot consist of only expressions). These
- * limitations help to keep the index scan similar to PK/RI index scans.
+ * the relation. The index must be btree, non-partial, and the leftmost
+ * field must be a column (not an expression) that reference the remote
+ * relation. These limitations help to keep the index scan similar to
+ * PK/RI index scans.
  *
  * Note that the limitations of index scans for replica identity full only
  * adheres to a subset of the limitations of PK/RI. For example, we support
@@ -796,10 +797,6 @@ RemoteRelContainsLeftMostColumnOnIdx(IndexInfo *indexInfo, AttrMap *attrmap)
  * none of the tuples satisfy the expression for the index scan, we fall-back
  * to sequential execution, which might not be a good idea in some cases.
  *
- * We also skip indexes if the remote relation does not contain the leftmost
- * column of the index. This is because in most such cases sequential scan is
- * favorable over index scan.
- *
  * We expect to call this function when REPLICA IDENTITY FULL is defined for
  * the remote relation.
  *
-- 
2.31.1

From 1afcd0a67b2d49840821d6e62fc38de9253121de Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Fri, 7 Jul 2023 16:49:39 +0900
Subject: [PATCH v5] Doc: clarify the conditions of usable indexes for REPLICA
 IDENTITY FULL tables.

Commit 89e46da5e allowed REPLICA IDENTIFY FULL tables to use an index
on the subscriber during apply of update/delete.

The source code comments are also updated accordingly.

Reviewed-by: Amit Kapila, Peter Smith
Discussion: https://postgr.es/m/CAD21AoDJjffEvUFKXT27Q5U8-UU9JHv4rrJ9Ke8Zkc5UPWHLvA@mail.gmail.com
Backpatch-through: 16
---
 doc/src/sgml/logical-replication.sgml      | 12 ++++++------
 src/backend/executor/execReplication.c     |  6 +++---
 src/backend/replication/logical/relation.c | 11 ++++-------
 3 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index c5de2040f7..e0996b84ea 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -134,12 +134,12 @@
    to replica identity <literal>FULL</literal>, which means the entire row becomes
    the key.  When replica identity <literal>FULL</literal> is specified,
    indexes can be used on the subscriber side for searching the rows.  Candidate
-   indexes must be btree, non-partial, and have at least one column reference
-   (i.e. cannot consist of only expressions).  These restrictions
-   on the non-unique index properties adhere to some of the restrictions that
-   are enforced for primary keys.  If there are no such suitable indexes,
-   the search on the subscriber side can be very inefficient, therefore
-   replica identity <literal>FULL</literal> should only be used as a
+   indexes must be btree, non-partial, and the leftmost index field must be a
+   column (not an expression) that reference a published table column.  These
+   restrictions on the non-unique index properties adhere to some of the
+   restrictions that are enforced for primary keys.  If there are no such
+   suitable indexes, the search on the subscriber side can be very inefficient,
+   therefore replica identity <literal>FULL</literal> should only be used as a
    fallback if no other solution is possible.  If a replica identity other
    than <literal>FULL</literal> is set on the publisher side, a replica identity
    comprising the same or fewer columns must also be set on the subscriber
diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c
index 9dd7168461..af09342881 100644
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -47,9 +47,9 @@ static bool tuples_equal(TupleTableSlot *slot1, TupleTableSlot *slot2,
  *
  * Returns how many columns to use for the index scan.
  *
- * This is not generic routine, it expects the idxrel to be a btree, non-partial
- * and have at least one column reference (i.e. cannot consist of only
- * expressions).
+ * This is not generic routine, idxrel must be PK, RI, or an index that can be
+ * used for REPLICA IDENTITY FULL table. See FindUsableIndexForReplicaIdentityFull()
+ * for details.
  *
  * By definition, replication identity of a rel meets all limitations associated
  * with that. Note that any other index could also meet these limitations.
diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index 57ad22b48a..fe940335a3 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -779,9 +779,10 @@ RemoteRelContainsLeftMostColumnOnIdx(IndexInfo *indexInfo, AttrMap *attrmap)
 
 /*
  * Returns the oid of an index that can be used by the apply worker to scan
- * the relation. The index must be btree, non-partial, and have at least
- * one column reference (i.e. cannot consist of only expressions). These
- * limitations help to keep the index scan similar to PK/RI index scans.
+ * the relation. The index must be btree, non-partial, and the leftmost
+ * field must be a column (not an expression) that reference the remote
+ * relation. These limitations help to keep the index scan similar to
+ * PK/RI index scans.
  *
  * Note that the limitations of index scans for replica identity full only
  * adheres to a subset of the limitations of PK/RI. For example, we support
@@ -796,10 +797,6 @@ RemoteRelContainsLeftMostColumnOnIdx(IndexInfo *indexInfo, AttrMap *attrmap)
  * none of the tuples satisfy the expression for the index scan, we fall-back
  * to sequential execution, which might not be a good idea in some cases.
  *
- * We also skip indexes if the remote relation does not contain the leftmost
- * column of the index. This is because in most such cases sequential scan is
- * favorable over index scan.
- *
  * We expect to call this function when REPLICA IDENTITY FULL is defined for
  * the remote relation.
  *
-- 
2.31.1

diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c
index af09342881..25d2868744 100644
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -175,16 +175,7 @@ retry:
 		if (!isIdxSafeToSkipDuplicates)
 		{
 			if (eq == NULL)
-			{
-#ifdef USE_ASSERT_CHECKING
-				/* apply assertions only once for the input idxoid */
-				IndexInfo  *indexInfo = BuildIndexInfo(idxrel);
-
-				Assert(IsIndexUsableForReplicaIdentityFull(indexInfo));
-#endif
-
 				eq = palloc0(sizeof(*eq) * outslot->tts_tupleDescriptor->natts);
-			}
 
 			if (!tuples_equal(outslot, searchslot, eq))
 				continue;
diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index fe940335a3..39aedf24c8 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -731,52 +731,6 @@ logicalrep_partition_open(LogicalRepRelMapEntry *root,
 	return entry;
 }
 
-/*
- * Returns true if the given index consists only of expressions such as:
- * 	CREATE INDEX idx ON table(foo(col));
- *
- * Returns false even if there is one column reference:
- * 	 CREATE INDEX idx ON table(foo(col), col_2);
- */
-static bool
-IsIndexOnlyOnExpression(IndexInfo *indexInfo)
-{
-	for (int i = 0; i < indexInfo->ii_NumIndexKeyAttrs; i++)
-	{
-		AttrNumber	attnum = indexInfo->ii_IndexAttrNumbers[i];
-
-		if (AttributeNumberIsValid(attnum))
-			return false;
-	}
-
-	return true;
-}
-
-/*
- * Returns true if the attrmap contains the leftmost column of the index.
- * Otherwise returns false.
- *
- * attrmap is a map of local attributes to remote ones. We can consult this
- * map to check whether the local index attribute has a corresponding remote
- * attribute.
- */
-static bool
-RemoteRelContainsLeftMostColumnOnIdx(IndexInfo *indexInfo, AttrMap *attrmap)
-{
-	AttrNumber	keycol;
-
-	Assert(indexInfo->ii_NumIndexAttrs >= 1);
-
-	keycol = indexInfo->ii_IndexAttrNumbers[0];
-	if (!AttributeNumberIsValid(keycol))
-		return false;
-
-	if (attrmap->maplen <= AttrNumberGetAttrOffset(keycol))
-		return false;
-
-	return attrmap->attnums[AttrNumberGetAttrOffset(keycol)] >= 0;
-}
-
 /*
  * Returns the oid of an index that can be used by the apply worker to scan
  * the relation. The index must be btree, non-partial, and the leftmost
@@ -791,6 +745,10 @@ RemoteRelContainsLeftMostColumnOnIdx(IndexInfo *indexInfo, AttrMap *attrmap)
  * compare the tuples for non-PK/RI index scans. See
  * RelationFindReplTupleByIndex().
  *
+ * attrmap is a map of local attributes to remote ones. We can consult this
+ * map to check whether the local index attribute has a corresponding remote
+ * attribute.
+ *
  * XXX: There are no fundamental problems for supporting non-btree indexes.
  * We mostly need to relax the limitations in RelationFindReplTupleByIndex().
  * For partial indexes, the required changes are likely to be larger. If
@@ -811,40 +769,44 @@ FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap)
 	foreach(lc, idxlist)
 	{
 		Oid			idxoid = lfirst_oid(lc);
-		bool		isUsableIdx;
-		bool		containsLeftMostCol;
 		Relation	idxRel;
 		IndexInfo  *idxInfo;
+		AttrNumber	keycol;
 
 		idxRel = index_open(idxoid, AccessShareLock);
 		idxInfo = BuildIndexInfo(idxRel);
-		isUsableIdx = IsIndexUsableForReplicaIdentityFull(idxInfo);
-		containsLeftMostCol =
-			RemoteRelContainsLeftMostColumnOnIdx(idxInfo, attrmap);
 		index_close(idxRel, AccessShareLock);
 
+		/* Must be a btree index */
+		if (idxInfo->ii_Am != BTREE_AM_OID)
+			continue;
+
+		/* Must be non-partial index */
+		if (idxInfo->ii_Predicate != NIL)
+			continue;
+
+		Assert(idxInfo->ii_NumIndexAttrs >= 1);
+
+		/* The left most index field must not be an expression */
+		keycol = idxInfo->ii_IndexAttrNumbers[0];
+		if (!AttributeNumberIsValid(keycol))
+			continue;
+
+		/*
+		 * The lest most index field must have a reference to a remote
+		 * relation column.
+		 */
+		if (attrmap->maplen <= AttrNumberGetAttrOffset(keycol) ||
+			(attrmap->attnums[AttrNumberGetAttrOffset(keycol)] < 0))
+			continue;
+
 		/* Return the first eligible index found */
-		if (isUsableIdx && containsLeftMostCol)
-			return idxoid;
+		return idxoid;
 	}
 
 	return InvalidOid;
 }
 
-/*
- * Returns true if the index is usable for replica identity full. For details,
- * see FindUsableIndexForReplicaIdentityFull.
- */
-bool
-IsIndexUsableForReplicaIdentityFull(IndexInfo *indexInfo)
-{
-	bool		is_btree = (indexInfo->ii_Am == BTREE_AM_OID);
-	bool		is_partial = (indexInfo->ii_Predicate != NIL);
-	bool		is_only_on_expression = IsIndexOnlyOnExpression(indexInfo);
-
-	return is_btree && !is_partial && !is_only_on_expression;
-}
-
 /*
  * Get replica identity index or if it is not defined a primary key.
  *

From 267c54b68dccb966ad2ed82c22ece2a0e8a438d1 Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <msawada@postgresql.org>
Date: Thu, 13 Jul 2023 10:23:52 +0900
Subject: [PATCH v6] Doc: clarify the conditions of usable indexes for REPLICA
 IDENTITY FULL tables.

Commit 89e46da5e allowed REPLICA IDENTITY FULL tables to use an index
on the subscriber during apply of update/delete. This commit clarifies
in the documentation that the leftmost field of candidate indexes must
be a column (not an expression) that references the published relation
column.

The source code comments are also updated accordingly.

Reviewed-by: Peter Smith, Amit Kapila
Discussion: https://postgr.es/m/CAD21AoDJjffEvUFKXT27Q5U8-UU9JHv4rrJ9Ke8Zkc5UPWHLvA@mail.gmail.com
Backpatch-through: 16
---
 doc/src/sgml/logical-replication.sgml      | 12 ++++++------
 src/backend/executor/execReplication.c     |  6 +++---
 src/backend/replication/logical/relation.c | 11 ++++-------
 3 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index c5de2040f7..c2a749d882 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -134,12 +134,12 @@
    to replica identity <literal>FULL</literal>, which means the entire row becomes
    the key.  When replica identity <literal>FULL</literal> is specified,
    indexes can be used on the subscriber side for searching the rows.  Candidate
-   indexes must be btree, non-partial, and have at least one column reference
-   (i.e. cannot consist of only expressions).  These restrictions
-   on the non-unique index properties adhere to some of the restrictions that
-   are enforced for primary keys.  If there are no such suitable indexes,
-   the search on the subscriber side can be very inefficient, therefore
-   replica identity <literal>FULL</literal> should only be used as a
+   indexes must be btree, non-partial, and the leftmost index field must be a
+   column (not an expression) that references the published table column.  These
+   restrictions on the non-unique index properties adhere to some of the
+   restrictions that are enforced for primary keys.  If there are no such
+   suitable indexes, the search on the subscriber side can be very inefficient,
+   therefore replica identity <literal>FULL</literal> should only be used as a
    fallback if no other solution is possible.  If a replica identity other
    than <literal>FULL</literal> is set on the publisher side, a replica identity
    comprising the same or fewer columns must also be set on the subscriber
diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c
index 9dd7168461..af09342881 100644
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -47,9 +47,9 @@ static bool tuples_equal(TupleTableSlot *slot1, TupleTableSlot *slot2,
  *
  * Returns how many columns to use for the index scan.
  *
- * This is not generic routine, it expects the idxrel to be a btree, non-partial
- * and have at least one column reference (i.e. cannot consist of only
- * expressions).
+ * This is not generic routine, idxrel must be PK, RI, or an index that can be
+ * used for REPLICA IDENTITY FULL table. See FindUsableIndexForReplicaIdentityFull()
+ * for details.
  *
  * By definition, replication identity of a rel meets all limitations associated
  * with that. Note that any other index could also meet these limitations.
diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index 57ad22b48a..c545b90636 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -779,9 +779,10 @@ RemoteRelContainsLeftMostColumnOnIdx(IndexInfo *indexInfo, AttrMap *attrmap)
 
 /*
  * Returns the oid of an index that can be used by the apply worker to scan
- * the relation. The index must be btree, non-partial, and have at least
- * one column reference (i.e. cannot consist of only expressions). These
- * limitations help to keep the index scan similar to PK/RI index scans.
+ * the relation. The index must be btree, non-partial, and the leftmost
+ * field must be a column (not an expression) that references the remote
+ * relation column. These limitations help to keep the index scan similar
+ * to PK/RI index scans.
  *
  * Note that the limitations of index scans for replica identity full only
  * adheres to a subset of the limitations of PK/RI. For example, we support
@@ -796,10 +797,6 @@ RemoteRelContainsLeftMostColumnOnIdx(IndexInfo *indexInfo, AttrMap *attrmap)
  * none of the tuples satisfy the expression for the index scan, we fall-back
  * to sequential execution, which might not be a good idea in some cases.
  *
- * We also skip indexes if the remote relation does not contain the leftmost
- * column of the index. This is because in most such cases sequential scan is
- * favorable over index scan.
- *
  * We expect to call this function when REPLICA IDENTITY FULL is defined for
  * the remote relation.
  *
-- 
2.31.1

diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c
index 9dd7168461..e2b29cd9bd 100644
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -132,7 +132,7 @@ build_replindex_scan_key(ScanKey skey, Relation rel, Relation idxrel,
  * contents, and return true.  Return false otherwise.
  */
 bool
-RelationFindReplTupleByIndex(Relation rel, Oid idxoid,
+RelationFindReplTupleByIndex(Relation rel, Relation idxrel,
 							 LockTupleMode lockmode,
 							 TupleTableSlot *searchslot,
 							 TupleTableSlot *outslot)
@@ -142,15 +142,12 @@ RelationFindReplTupleByIndex(Relation rel, Oid idxoid,
 	IndexScanDesc scan;
 	SnapshotData snap;
 	TransactionId xwait;
-	Relation	idxrel;
 	bool		found;
 	TypeCacheEntry **eq = NULL;
 	bool		isIdxSafeToSkipDuplicates;
 
-	/* Open the index. */
-	idxrel = index_open(idxoid, RowExclusiveLock);
-
-	isIdxSafeToSkipDuplicates = (GetRelationIdentityOrPK(rel) == idxoid);
+	isIdxSafeToSkipDuplicates =
+		(GetRelationIdentityOrPK(rel) == RelationGetRelid(idxrel));
 
 	InitDirtySnapshot(snap);
 
@@ -175,16 +172,7 @@ retry:
 		if (!isIdxSafeToSkipDuplicates)
 		{
 			if (eq == NULL)
-			{
-#ifdef USE_ASSERT_CHECKING
-				/* apply assertions only once for the input idxoid */
-				IndexInfo  *indexInfo = BuildIndexInfo(idxrel);
-
-				Assert(IsIndexUsableForReplicaIdentityFull(indexInfo));
-#endif
-
 				eq = palloc0(sizeof(*eq) * outslot->tts_tupleDescriptor->natts);
-			}
 
 			if (!tuples_equal(outslot, searchslot, eq))
 				continue;
@@ -260,9 +248,6 @@ retry:
 
 	index_endscan(scan);
 
-	/* Don't release lock until commit. */
-	index_close(idxrel, NoLock);
-
 	return found;
 }
 
diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index 57ad22b48a..57a5de44b1 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -731,52 +731,6 @@ logicalrep_partition_open(LogicalRepRelMapEntry *root,
 	return entry;
 }
 
-/*
- * Returns true if the given index consists only of expressions such as:
- * 	CREATE INDEX idx ON table(foo(col));
- *
- * Returns false even if there is one column reference:
- * 	 CREATE INDEX idx ON table(foo(col), col_2);
- */
-static bool
-IsIndexOnlyOnExpression(IndexInfo *indexInfo)
-{
-	for (int i = 0; i < indexInfo->ii_NumIndexKeyAttrs; i++)
-	{
-		AttrNumber	attnum = indexInfo->ii_IndexAttrNumbers[i];
-
-		if (AttributeNumberIsValid(attnum))
-			return false;
-	}
-
-	return true;
-}
-
-/*
- * Returns true if the attrmap contains the leftmost column of the index.
- * Otherwise returns false.
- *
- * attrmap is a map of local attributes to remote ones. We can consult this
- * map to check whether the local index attribute has a corresponding remote
- * attribute.
- */
-static bool
-RemoteRelContainsLeftMostColumnOnIdx(IndexInfo *indexInfo, AttrMap *attrmap)
-{
-	AttrNumber	keycol;
-
-	Assert(indexInfo->ii_NumIndexAttrs >= 1);
-
-	keycol = indexInfo->ii_IndexAttrNumbers[0];
-	if (!AttributeNumberIsValid(keycol))
-		return false;
-
-	if (attrmap->maplen <= AttrNumberGetAttrOffset(keycol))
-		return false;
-
-	return attrmap->attnums[AttrNumberGetAttrOffset(keycol)] >= 0;
-}
-
 /*
  * Returns the oid of an index that can be used by the apply worker to scan
  * the relation. The index must be btree, non-partial, and have at least
@@ -815,19 +769,16 @@ FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap)
 	{
 		Oid			idxoid = lfirst_oid(lc);
 		bool		isUsableIdx;
-		bool		containsLeftMostCol;
 		Relation	idxRel;
 		IndexInfo  *idxInfo;
 
 		idxRel = index_open(idxoid, AccessShareLock);
 		idxInfo = BuildIndexInfo(idxRel);
-		isUsableIdx = IsIndexUsableForReplicaIdentityFull(idxInfo);
-		containsLeftMostCol =
-			RemoteRelContainsLeftMostColumnOnIdx(idxInfo, attrmap);
+		isUsableIdx = IsIndexUsableForReplicaIdentityFull(idxInfo, attrmap);
 		index_close(idxRel, AccessShareLock);
 
 		/* Return the first eligible index found */
-		if (isUsableIdx && containsLeftMostCol)
+		if (isUsableidx)
 			return idxoid;
 	}
 
@@ -837,15 +788,35 @@ FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap)
 /*
  * Returns true if the index is usable for replica identity full. For details,
  * see FindUsableIndexForReplicaIdentityFull.
+ *
+ * attrmap is a map of local attributes to remote ones. We can consult this
+ * map to check whether the local index attribute has a corresponding remote
+ * attribute.
  */
 bool
-IsIndexUsableForReplicaIdentityFull(IndexInfo *indexInfo)
+IsIndexUsableForReplicaIdentityFull(IndexInfo *indexInfo, AttrMap *attrmap)
 {
 	bool		is_btree = (indexInfo->ii_Am == BTREE_AM_OID);
 	bool		is_partial = (indexInfo->ii_Predicate != NIL);
-	bool		is_only_on_expression = IsIndexOnlyOnExpression(indexInfo);
+	AttrNumber	keycol;
 
-	return is_btree && !is_partial && !is_only_on_expression;
+	/* Must be btree and non-partial index */
+	if (!is_btree || is_partial)
+		return false;
+
+	Assert(indexInfo->ii_NumIndexAttrs >= 1);
+
+	/* The leftmost index field must not be an expression */
+	keycol = indexInfo->ii_IndexAttrNumbers[0];
+	if (!AttributeNumberIsValid(keycol))
+		return false;
+
+	/* And must reference the remote relation column */
+	if (attrmap->maplen <= AttrNumberGetAttrOffset(keycol) ||
+		attrmap->attnums[AttrNumberGetAttrOffset(keycol)] < 0)
+		return false;
+
+	return true;
 }
 
 /*
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 0ee764d68f..6c09334ffc 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -78,7 +78,7 @@
  *
  * To avoid this, and similar prepare confusions the subscription's two_phase
  * commit is enabled only after the initial sync is over. The two_phase option
- * has been implemented as a tri-state with values DISABLED, PENDING, and
+n * has been implemented as a tri-state with values DISABLED, PENDING, and
  * ENABLED.
  *
  * Even if the user specifies they want a subscription with two_phase = on,
@@ -140,6 +140,7 @@
 #include <sys/stat.h>
 #include <unistd.h>
 
+#include "access/genam.h"
 #include "access/table.h"
 #include "access/tableam.h"
 #include "access/twophase.h"
@@ -410,7 +411,7 @@ static void apply_handle_delete_internal(ApplyExecutionData *edata,
 										 ResultRelInfo *relinfo,
 										 TupleTableSlot *remoteslot,
 										 Oid localindexoid);
-static bool FindReplTupleInLocalRel(EState *estate, Relation localrel,
+static bool FindReplTupleInLocalRel(ApplyExecutionData *edata, Relation localrel,
 									LogicalRepRelation *remoterel,
 									Oid localidxoid,
 									TupleTableSlot *remoteslot,
@@ -2677,7 +2678,7 @@ apply_handle_update_internal(ApplyExecutionData *edata,
 	EvalPlanQualInit(&epqstate, estate, NULL, NIL, -1, NIL);
 	ExecOpenIndices(relinfo, false);
 
-	found = FindReplTupleInLocalRel(estate, localrel,
+	found = FindReplTupleInLocalRel(edata, localrel,
 									&relmapentry->remoterel,
 									localindexoid,
 									remoteslot, &localslot);
@@ -2830,7 +2831,7 @@ apply_handle_delete_internal(ApplyExecutionData *edata,
 	EvalPlanQualInit(&epqstate, estate, NULL, NIL, -1, NIL);
 	ExecOpenIndices(relinfo, false);
 
-	found = FindReplTupleInLocalRel(estate, localrel, remoterel, localindexoid,
+	found = FindReplTupleInLocalRel(edata, localrel, remoterel, localindexoid,
 									remoteslot, &localslot);
 
 	/* If found delete it. */
@@ -2869,12 +2870,13 @@ apply_handle_delete_internal(ApplyExecutionData *edata,
  * Local tuple, if found, is returned in '*localslot'.
  */
 static bool
-FindReplTupleInLocalRel(EState *estate, Relation localrel,
+FindReplTupleInLocalRel(ApplyExecutionData *edata, Relation localrel,
 						LogicalRepRelation *remoterel,
 						Oid localidxoid,
 						TupleTableSlot *remoteslot,
 						TupleTableSlot **localslot)
 {
+	EState	   *estate = edata->estate;
 	bool		found;
 
 	/*
@@ -2889,9 +2891,24 @@ FindReplTupleInLocalRel(EState *estate, Relation localrel,
 		   (remoterel->replident == REPLICA_IDENTITY_FULL));
 
 	if (OidIsValid(localidxoid))
-		found = RelationFindReplTupleByIndex(localrel, localidxoid,
+	{
+		Relation	idxrel;
+
+		/* Open the index. */
+		idxrel = index_open(localidxoid, RowExclusiveLock);
+
+		/* Index must be PK, RI, or usable for REPLICA IDENTITY FULL tables */
+		Assert(GetRelationIdentityOrPK(idxrel) == localidxoid ||
+			   IsIndexUsableForReplicaIdentityFull(BuildIndexInfo(idxrel),
+												   edata->targetRel->attrmap));
+
+		found = RelationFindReplTupleByIndex(localrel, idxrel,
 											 LockTupleExclusive,
 											 remoteslot, *localslot);
+
+		/* Don't release lock until commit. */
+		index_close(idxrel, NoLock);
+	}
 	else
 		found = RelationFindReplTupleSeq(localrel, LockTupleExclusive,
 										 remoteslot, *localslot);
@@ -3009,7 +3026,7 @@ apply_handle_tuple_routing(ApplyExecutionData *edata,
 				bool		found;
 
 				/* Get the matching local tuple from the partition. */
-				found = FindReplTupleInLocalRel(estate, partrel,
+				found = FindReplTupleInLocalRel(edata, partrel,
 												&part_entry->remoterel,
 												part_entry->localindexoid,
 												remoteslot_part, &localslot);
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index ac02247947..92153da1df 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -646,7 +646,7 @@ extern void check_exclusion_constraint(Relation heap, Relation index,
 /*
  * prototypes from functions in execReplication.c
  */
-extern bool RelationFindReplTupleByIndex(Relation rel, Oid idxoid,
+extern bool RelationFindReplTupleByIndex(Relation rel, Relation idxrel,
 										 LockTupleMode lockmode,
 										 TupleTableSlot *searchslot,
 										 TupleTableSlot *outslot);
diff --git a/src/include/replication/logicalrelation.h b/src/include/replication/logicalrelation.h
index 921b9974db..3f4d906d74 100644
--- a/src/include/replication/logicalrelation.h
+++ b/src/include/replication/logicalrelation.h
@@ -48,7 +48,7 @@ extern LogicalRepRelMapEntry *logicalrep_partition_open(LogicalRepRelMapEntry *r
 														Relation partrel, AttrMap *map);
 extern void logicalrep_rel_close(LogicalRepRelMapEntry *rel,
 								 LOCKMODE lockmode);
-extern bool IsIndexUsableForReplicaIdentityFull(IndexInfo *indexInfo);
+extern bool IsIndexUsableForReplicaIdentityFull(IndexInfo *indexInfo, AttrMap *attrmap);
 extern Oid	GetRelationIdentityOrPK(Relation rel);
 
 #endif							/* LOGICALRELATION_H */

From da4eb0de3eda7792b1f8fd0de7ff19e4dfeadba5 Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Tue, 18 Jul 2023 15:11:27 +0900
Subject: [PATCH v3] Remove unnecessary checks for indexes for REPLICA IDENTITY
 FULL tables.

Previously, when selecting an usable index for update/delete for the
REPLICA IDENTITY FULL table, in IsIndexonlyExpression(), we used to
check if all index fields are not expressions. However, it was not
necessary actually, because it is enough to check if only the leftmost
index field is not an expression (and references the remote table
column) and this check has already been checked by
RemoteRelContainsLeftMostColumnOnIdx().

This commit removes IsIndexOnlyExpression() and
RemoteRelContainsLeftMostColumnOnIdx() and all checks for usable
indexes for REPLICA IDENTITY FULL tables are now performed by
IsIndexUsableForReplicaIdentityFull().

Reviewed-by:
Discussion: https://postgr.es/m/
---
 src/backend/executor/execReplication.c     |  9 ---
 src/backend/replication/logical/relation.c | 92 ++++++++--------------
 src/backend/replication/logical/worker.c   | 24 ++++--
 src/include/replication/logicalrelation.h  |  2 +-
 4 files changed, 51 insertions(+), 76 deletions(-)

diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c
index e776524227..81f27042bc 100644
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -222,16 +222,7 @@ retry:
 		if (!isIdxSafeToSkipDuplicates)
 		{
 			if (eq == NULL)
-			{
-#ifdef USE_ASSERT_CHECKING
-				/* apply assertions only once for the input idxoid */
-				IndexInfo  *indexInfo = BuildIndexInfo(idxrel);
-
-				Assert(IsIndexUsableForReplicaIdentityFull(indexInfo));
-#endif
-
 				eq = palloc0(sizeof(*eq) * outslot->tts_tupleDescriptor->natts);
-			}
 
 			if (!tuples_equal(outslot, searchslot, eq))
 				continue;
diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index ed57e5d2b6..ad960f7bad 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -734,58 +734,9 @@ logicalrep_partition_open(LogicalRepRelMapEntry *root,
 	return entry;
 }
 
-/*
- * Returns true if the given index consists only of expressions such as:
- * 	CREATE INDEX idx ON table(foo(col));
- *
- * Returns false even if there is one column reference:
- * 	 CREATE INDEX idx ON table(foo(col), col_2);
- */
-static bool
-IsIndexOnlyOnExpression(IndexInfo *indexInfo)
-{
-	for (int i = 0; i < indexInfo->ii_NumIndexKeyAttrs; i++)
-	{
-		AttrNumber	attnum = indexInfo->ii_IndexAttrNumbers[i];
-
-		if (AttributeNumberIsValid(attnum))
-			return false;
-	}
-
-	return true;
-}
-
-/*
- * Returns true if the attrmap contains the leftmost column of the index.
- * Otherwise returns false.
- *
- * attrmap is a map of local attributes to remote ones. We can consult this
- * map to check whether the local index attribute has a corresponding remote
- * attribute.
- */
-static bool
-RemoteRelContainsLeftMostColumnOnIdx(IndexInfo *indexInfo, AttrMap *attrmap)
-{
-	AttrNumber	keycol;
-
-	Assert(indexInfo->ii_NumIndexAttrs >= 1);
-
-	keycol = indexInfo->ii_IndexAttrNumbers[0];
-	if (!AttributeNumberIsValid(keycol))
-		return false;
-
-	if (attrmap->maplen <= AttrNumberGetAttrOffset(keycol))
-		return false;
-
-	return attrmap->attnums[AttrNumberGetAttrOffset(keycol)] >= 0;
-}
-
 /*
  * Returns the oid of an index that can be used by the apply worker to scan
- * the relation. The index must be btree or hash, non-partial, and the leftmost
- * field must be a column (not an expression) that references the remote
- * relation column. These limitations help to keep the index scan similar
- * to PK/RI index scans.
+ * the relation.
  *
  * Note that the limitations of index scans for replica identity full only
  * adheres to a subset of the limitations of PK/RI. For example, we support
@@ -815,19 +766,16 @@ FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap)
 	{
 		Oid			idxoid = lfirst_oid(lc);
 		bool		isUsableIdx;
-		bool		containsLeftMostCol;
 		Relation	idxRel;
 		IndexInfo  *idxInfo;
 
 		idxRel = index_open(idxoid, AccessShareLock);
 		idxInfo = BuildIndexInfo(idxRel);
-		isUsableIdx = IsIndexUsableForReplicaIdentityFull(idxInfo);
-		containsLeftMostCol =
-			RemoteRelContainsLeftMostColumnOnIdx(idxInfo, attrmap);
+		isUsableIdx = IsIndexUsableForReplicaIdentityFull(idxInfo, attrmap);
 		index_close(idxRel, AccessShareLock);
 
 		/* Return the first eligible index found */
-		if (isUsableIdx && containsLeftMostCol)
+		if (isUsableIdx)
 			return idxoid;
 	}
 
@@ -835,11 +783,13 @@ FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap)
 }
 
 /*
- * Returns true if the index is usable for replica identity full. For details,
- * see FindUsableIndexForReplicaIdentityFull.
+ * Returns true if the index is usable for replica identity full.
+ *
+ * The index must be btree or hash, non-partial, and the leftmost field must be
+ * a column (not an expression) that references the remote relation column. These
+ * limitations help to keep the index scan similar to PK/RI index scans.
  *
- * Currently, only Btree and Hash indexes can be returned as usable. This
- * is due to following reasons:
+ * The reasons why only Btree and Hash indexes can be considered as usable are:
  *
  * 1) Other index access methods don't have a fixed strategy for equality
  * operation. Refer get_equal_strategy_number_for_am().
@@ -851,16 +801,36 @@ FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap)
  *
  * XXX: Note that BRIN and GIN indexes do not implement "amgettuple" which
  * will be used later to fetch the tuples. See RelationFindReplTupleByIndex().
+ *
+ * attrmap is a map of local attributes to remote ones. We can consult this
+ * map to check whether the local index attribute has a corresponding remote
+ * attribute.
  */
 bool
-IsIndexUsableForReplicaIdentityFull(IndexInfo *indexInfo)
+IsIndexUsableForReplicaIdentityFull(IndexInfo *indexInfo, AttrMap *attrmap)
 {
+	AttrNumber	keycol;
+
 	/* Ensure that the index access method has a valid equal strategy */
 	if (get_equal_strategy_number_for_am(indexInfo->ii_Am) == InvalidStrategy)
 		return false;
 	if (indexInfo->ii_Predicate != NIL)
 		return false;
-	if (IsIndexOnlyOnExpression(indexInfo))
+
+	Assert(indexInfo->ii_NumIndexAttrs >= 1);
+
+	/* The leftmost index field must not be an expression */
+	keycol = indexInfo->ii_IndexAttrNumbers[0];
+	if (!AttributeNumberIsValid(keycol))
+		return false;
+
+	/*
+	 * And must reference the remote relation column. This is because if it
+	 * doesn't, the sequential scan is favorable over index scan in most
+	 * cases..
+	 */
+	if (attrmap->maplen <= AttrNumberGetAttrOffset(keycol) ||
+		attrmap->attnums[AttrNumberGetAttrOffset(keycol)] < 0)
 		return false;
 
 #ifdef USE_ASSERT_CHECKING
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index dd353fd1cb..0dd69739fb 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -140,6 +140,7 @@
 #include <sys/stat.h>
 #include <unistd.h>
 
+#include "access/genam.h"
 #include "access/table.h"
 #include "access/tableam.h"
 #include "access/twophase.h"
@@ -410,7 +411,7 @@ static void apply_handle_delete_internal(ApplyExecutionData *edata,
 										 ResultRelInfo *relinfo,
 										 TupleTableSlot *remoteslot,
 										 Oid localindexoid);
-static bool FindReplTupleInLocalRel(EState *estate, Relation localrel,
+static bool FindReplTupleInLocalRel(ApplyExecutionData *edata, Relation localrel,
 									LogicalRepRelation *remoterel,
 									Oid localidxoid,
 									TupleTableSlot *remoteslot,
@@ -2663,7 +2664,7 @@ apply_handle_update_internal(ApplyExecutionData *edata,
 	EvalPlanQualInit(&epqstate, estate, NULL, NIL, -1, NIL);
 	ExecOpenIndices(relinfo, false);
 
-	found = FindReplTupleInLocalRel(estate, localrel,
+	found = FindReplTupleInLocalRel(edata, localrel,
 									&relmapentry->remoterel,
 									localindexoid,
 									remoteslot, &localslot);
@@ -2816,7 +2817,7 @@ apply_handle_delete_internal(ApplyExecutionData *edata,
 	EvalPlanQualInit(&epqstate, estate, NULL, NIL, -1, NIL);
 	ExecOpenIndices(relinfo, false);
 
-	found = FindReplTupleInLocalRel(estate, localrel, remoterel, localindexoid,
+	found = FindReplTupleInLocalRel(edata, localrel, remoterel, localindexoid,
 									remoteslot, &localslot);
 
 	/* If found delete it. */
@@ -2855,12 +2856,13 @@ apply_handle_delete_internal(ApplyExecutionData *edata,
  * Local tuple, if found, is returned in '*localslot'.
  */
 static bool
-FindReplTupleInLocalRel(EState *estate, Relation localrel,
+FindReplTupleInLocalRel(ApplyExecutionData *edata, Relation localrel,
 						LogicalRepRelation *remoterel,
 						Oid localidxoid,
 						TupleTableSlot *remoteslot,
 						TupleTableSlot **localslot)
 {
+	EState	   *estate = edata->estate;
 	bool		found;
 
 	/*
@@ -2875,9 +2877,21 @@ FindReplTupleInLocalRel(EState *estate, Relation localrel,
 		   (remoterel->replident == REPLICA_IDENTITY_FULL));
 
 	if (OidIsValid(localidxoid))
+	{
+#ifdef USE_ASSERT_CHECKING
+		Relation	idxrel = index_open(localidxoid, AccessShareLock);
+
+		/* Index must be PK, RI, or usable for REPLICA IDENTITY FULL tables */
+		Assert(GetRelationIdentityOrPK(idxrel) == localidxoid ||
+			   IsIndexUsableForReplicaIdentityFull(BuildIndexInfo(idxrel),
+												   edata->targetRel->attrmap));
+		index_close(idxrel, AccessShareLock);
+#endif
+
 		found = RelationFindReplTupleByIndex(localrel, localidxoid,
 											 LockTupleExclusive,
 											 remoteslot, *localslot);
+	}
 	else
 		found = RelationFindReplTupleSeq(localrel, LockTupleExclusive,
 										 remoteslot, *localslot);
@@ -2995,7 +3009,7 @@ apply_handle_tuple_routing(ApplyExecutionData *edata,
 				bool		found;
 
 				/* Get the matching local tuple from the partition. */
-				found = FindReplTupleInLocalRel(estate, partrel,
+				found = FindReplTupleInLocalRel(edata, partrel,
 												&part_entry->remoterel,
 												part_entry->localindexoid,
 												remoteslot_part, &localslot);
diff --git a/src/include/replication/logicalrelation.h b/src/include/replication/logicalrelation.h
index 921b9974db..3f4d906d74 100644
--- a/src/include/replication/logicalrelation.h
+++ b/src/include/replication/logicalrelation.h
@@ -48,7 +48,7 @@ extern LogicalRepRelMapEntry *logicalrep_partition_open(LogicalRepRelMapEntry *r
 														Relation partrel, AttrMap *map);
 extern void logicalrep_rel_close(LogicalRepRelMapEntry *rel,
 								 LOCKMODE lockmode);
-extern bool IsIndexUsableForReplicaIdentityFull(IndexInfo *indexInfo);
+extern bool IsIndexUsableForReplicaIdentityFull(IndexInfo *indexInfo, AttrMap *attrmap);
 extern Oid	GetRelationIdentityOrPK(Relation rel);
 
 #endif							/* LOGICALRELATION_H */
-- 
2.31.1

From 1fb3cdddc8a73bdafaca804e0877d97185472019 Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Tue, 18 Jul 2023 15:11:27 +0900
Subject: [PATCH v4] Remove unnecessary checks for indexes for REPLICA IDENTITY
 FULL tables.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Previously, when selecting an usable index for update/delete for the
REPLICA IDENTITY FULL table, in IsIndexOnlyExpression(), we used to
check if all index fields are not expressions. However, it was not
necessary, because it is enough to check if only the leftmost index
field is not an expression (and references the remote table column)
and this check has already been done by
RemoteRelContainsLeftMostColumnOnIdx().

This commit removes IsIndexOnlyExpression() and
RemoteRelContainsLeftMostColumnOnIdx() and all checks for usable
indexes for REPLICA IDENTITY FULL tables are now performed by
IsIndexUsableForReplicaIdentityFull().

Backpatch this to remain the code consistent.

Reported-by: Peter Smith
Reviewed-by: Amit Kapila, Önder Kalacı
Discussion: https://postgr.es/m/CAHut%2BPsGRE5WSsY0jcLHJEoA17MrbP9yy8FxdjC_ZOAACxbt%2BQ%40mail.gmail.com
Backpatch-through: 16
---
 src/backend/executor/execReplication.c     |  9 ---
 src/backend/replication/logical/relation.c | 94 ++++++++--------------
 src/backend/replication/logical/worker.c   | 24 ++++--
 src/include/replication/logicalrelation.h  |  2 +-
 4 files changed, 53 insertions(+), 76 deletions(-)

diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c
index e776524227..81f27042bc 100644
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -222,16 +222,7 @@ retry:
 		if (!isIdxSafeToSkipDuplicates)
 		{
 			if (eq == NULL)
-			{
-#ifdef USE_ASSERT_CHECKING
-				/* apply assertions only once for the input idxoid */
-				IndexInfo  *indexInfo = BuildIndexInfo(idxrel);
-
-				Assert(IsIndexUsableForReplicaIdentityFull(indexInfo));
-#endif
-
 				eq = palloc0(sizeof(*eq) * outslot->tts_tupleDescriptor->natts);
-			}
 
 			if (!tuples_equal(outslot, searchslot, eq))
 				continue;
diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index ed57e5d2b6..6ca9839fe0 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -734,58 +734,9 @@ logicalrep_partition_open(LogicalRepRelMapEntry *root,
 	return entry;
 }
 
-/*
- * Returns true if the given index consists only of expressions such as:
- * 	CREATE INDEX idx ON table(foo(col));
- *
- * Returns false even if there is one column reference:
- * 	 CREATE INDEX idx ON table(foo(col), col_2);
- */
-static bool
-IsIndexOnlyOnExpression(IndexInfo *indexInfo)
-{
-	for (int i = 0; i < indexInfo->ii_NumIndexKeyAttrs; i++)
-	{
-		AttrNumber	attnum = indexInfo->ii_IndexAttrNumbers[i];
-
-		if (AttributeNumberIsValid(attnum))
-			return false;
-	}
-
-	return true;
-}
-
-/*
- * Returns true if the attrmap contains the leftmost column of the index.
- * Otherwise returns false.
- *
- * attrmap is a map of local attributes to remote ones. We can consult this
- * map to check whether the local index attribute has a corresponding remote
- * attribute.
- */
-static bool
-RemoteRelContainsLeftMostColumnOnIdx(IndexInfo *indexInfo, AttrMap *attrmap)
-{
-	AttrNumber	keycol;
-
-	Assert(indexInfo->ii_NumIndexAttrs >= 1);
-
-	keycol = indexInfo->ii_IndexAttrNumbers[0];
-	if (!AttributeNumberIsValid(keycol))
-		return false;
-
-	if (attrmap->maplen <= AttrNumberGetAttrOffset(keycol))
-		return false;
-
-	return attrmap->attnums[AttrNumberGetAttrOffset(keycol)] >= 0;
-}
-
 /*
  * Returns the oid of an index that can be used by the apply worker to scan
- * the relation. The index must be btree or hash, non-partial, and the leftmost
- * field must be a column (not an expression) that references the remote
- * relation column. These limitations help to keep the index scan similar
- * to PK/RI index scans.
+ * the relation.
  *
  * Note that the limitations of index scans for replica identity full only
  * adheres to a subset of the limitations of PK/RI. For example, we support
@@ -815,19 +766,16 @@ FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap)
 	{
 		Oid			idxoid = lfirst_oid(lc);
 		bool		isUsableIdx;
-		bool		containsLeftMostCol;
 		Relation	idxRel;
 		IndexInfo  *idxInfo;
 
 		idxRel = index_open(idxoid, AccessShareLock);
 		idxInfo = BuildIndexInfo(idxRel);
-		isUsableIdx = IsIndexUsableForReplicaIdentityFull(idxInfo);
-		containsLeftMostCol =
-			RemoteRelContainsLeftMostColumnOnIdx(idxInfo, attrmap);
+		isUsableIdx = IsIndexUsableForReplicaIdentityFull(idxInfo, attrmap);
 		index_close(idxRel, AccessShareLock);
 
 		/* Return the first eligible index found */
-		if (isUsableIdx && containsLeftMostCol)
+		if (isUsableIdx)
 			return idxoid;
 	}
 
@@ -835,11 +783,13 @@ FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap)
 }
 
 /*
- * Returns true if the index is usable for replica identity full. For details,
- * see FindUsableIndexForReplicaIdentityFull.
+ * Returns true if the index is usable for replica identity full.
  *
- * Currently, only Btree and Hash indexes can be returned as usable. This
- * is due to following reasons:
+ * The index must be btree or hash, non-partial, and the leftmost field must be
+ * a column (not an expression) that references the remote relation column. These
+ * limitations help to keep the index scan similar to PK/RI index scans.
+ *
+ * The reasons why only Btree and Hash indexes can be considered as usable are:
  *
  * 1) Other index access methods don't have a fixed strategy for equality
  * operation. Refer get_equal_strategy_number_for_am().
@@ -851,16 +801,38 @@ FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap)
  *
  * XXX: Note that BRIN and GIN indexes do not implement "amgettuple" which
  * will be used later to fetch the tuples. See RelationFindReplTupleByIndex().
+ *
+ * attrmap is a map of local attributes to remote ones. We can consult this
+ * map to check whether the local index attribute has a corresponding remote
+ * attribute.
  */
 bool
-IsIndexUsableForReplicaIdentityFull(IndexInfo *indexInfo)
+IsIndexUsableForReplicaIdentityFull(IndexInfo *indexInfo, AttrMap *attrmap)
 {
+	AttrNumber	keycol;
+
 	/* Ensure that the index access method has a valid equal strategy */
 	if (get_equal_strategy_number_for_am(indexInfo->ii_Am) == InvalidStrategy)
 		return false;
+
+	/* The index must not be a partial index */
 	if (indexInfo->ii_Predicate != NIL)
 		return false;
-	if (IsIndexOnlyOnExpression(indexInfo))
+
+	Assert(indexInfo->ii_NumIndexAttrs >= 1);
+
+	/* The leftmost index field must not be an expression */
+	keycol = indexInfo->ii_IndexAttrNumbers[0];
+	if (!AttributeNumberIsValid(keycol))
+		return false;
+
+	/*
+	 * And the leftmost index field must reference the remote relation column.
+	 * This is because if it doesn't, the sequential scan is favorable over
+	 * index scan in most cases.
+	 */
+	if (attrmap->maplen <= AttrNumberGetAttrOffset(keycol) ||
+		attrmap->attnums[AttrNumberGetAttrOffset(keycol)] < 0)
 		return false;
 
 #ifdef USE_ASSERT_CHECKING
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index dd353fd1cb..0dd69739fb 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -140,6 +140,7 @@
 #include <sys/stat.h>
 #include <unistd.h>
 
+#include "access/genam.h"
 #include "access/table.h"
 #include "access/tableam.h"
 #include "access/twophase.h"
@@ -410,7 +411,7 @@ static void apply_handle_delete_internal(ApplyExecutionData *edata,
 										 ResultRelInfo *relinfo,
 										 TupleTableSlot *remoteslot,
 										 Oid localindexoid);
-static bool FindReplTupleInLocalRel(EState *estate, Relation localrel,
+static bool FindReplTupleInLocalRel(ApplyExecutionData *edata, Relation localrel,
 									LogicalRepRelation *remoterel,
 									Oid localidxoid,
 									TupleTableSlot *remoteslot,
@@ -2663,7 +2664,7 @@ apply_handle_update_internal(ApplyExecutionData *edata,
 	EvalPlanQualInit(&epqstate, estate, NULL, NIL, -1, NIL);
 	ExecOpenIndices(relinfo, false);
 
-	found = FindReplTupleInLocalRel(estate, localrel,
+	found = FindReplTupleInLocalRel(edata, localrel,
 									&relmapentry->remoterel,
 									localindexoid,
 									remoteslot, &localslot);
@@ -2816,7 +2817,7 @@ apply_handle_delete_internal(ApplyExecutionData *edata,
 	EvalPlanQualInit(&epqstate, estate, NULL, NIL, -1, NIL);
 	ExecOpenIndices(relinfo, false);
 
-	found = FindReplTupleInLocalRel(estate, localrel, remoterel, localindexoid,
+	found = FindReplTupleInLocalRel(edata, localrel, remoterel, localindexoid,
 									remoteslot, &localslot);
 
 	/* If found delete it. */
@@ -2855,12 +2856,13 @@ apply_handle_delete_internal(ApplyExecutionData *edata,
  * Local tuple, if found, is returned in '*localslot'.
  */
 static bool
-FindReplTupleInLocalRel(EState *estate, Relation localrel,
+FindReplTupleInLocalRel(ApplyExecutionData *edata, Relation localrel,
 						LogicalRepRelation *remoterel,
 						Oid localidxoid,
 						TupleTableSlot *remoteslot,
 						TupleTableSlot **localslot)
 {
+	EState	   *estate = edata->estate;
 	bool		found;
 
 	/*
@@ -2875,9 +2877,21 @@ FindReplTupleInLocalRel(EState *estate, Relation localrel,
 		   (remoterel->replident == REPLICA_IDENTITY_FULL));
 
 	if (OidIsValid(localidxoid))
+	{
+#ifdef USE_ASSERT_CHECKING
+		Relation	idxrel = index_open(localidxoid, AccessShareLock);
+
+		/* Index must be PK, RI, or usable for REPLICA IDENTITY FULL tables */
+		Assert(GetRelationIdentityOrPK(idxrel) == localidxoid ||
+			   IsIndexUsableForReplicaIdentityFull(BuildIndexInfo(idxrel),
+												   edata->targetRel->attrmap));
+		index_close(idxrel, AccessShareLock);
+#endif
+
 		found = RelationFindReplTupleByIndex(localrel, localidxoid,
 											 LockTupleExclusive,
 											 remoteslot, *localslot);
+	}
 	else
 		found = RelationFindReplTupleSeq(localrel, LockTupleExclusive,
 										 remoteslot, *localslot);
@@ -2995,7 +3009,7 @@ apply_handle_tuple_routing(ApplyExecutionData *edata,
 				bool		found;
 
 				/* Get the matching local tuple from the partition. */
-				found = FindReplTupleInLocalRel(estate, partrel,
+				found = FindReplTupleInLocalRel(edata, partrel,
 												&part_entry->remoterel,
 												part_entry->localindexoid,
 												remoteslot_part, &localslot);
diff --git a/src/include/replication/logicalrelation.h b/src/include/replication/logicalrelation.h
index 921b9974db..3f4d906d74 100644
--- a/src/include/replication/logicalrelation.h
+++ b/src/include/replication/logicalrelation.h
@@ -48,7 +48,7 @@ extern LogicalRepRelMapEntry *logicalrep_partition_open(LogicalRepRelMapEntry *r
 														Relation partrel, AttrMap *map);
 extern void logicalrep_rel_close(LogicalRepRelMapEntry *rel,
 								 LOCKMODE lockmode);
-extern bool IsIndexUsableForReplicaIdentityFull(IndexInfo *indexInfo);
+extern bool IsIndexUsableForReplicaIdentityFull(IndexInfo *indexInfo, AttrMap *attrmap);
 extern Oid	GetRelationIdentityOrPK(Relation rel);
 
 #endif							/* LOGICALRELATION_H */
-- 
2.31.1

From 47578d5fb5cc9ec7690bb3c71e75b8461f5e7553 Mon Sep 17 00:00:00 2001
From: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Tue, 18 Jul 2023 15:11:27 +0900
Subject: [PATCH v5] Remove unnecessary checks for indexes for REPLICA IDENTITY
 FULL tables.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Previously, when selecting an usable index for update/delete for the
REPLICA IDENTITY FULL table, in IsIndexOnlyExpression(), we used to
check if all index fields are not expressions. However, it was not
necessary, because it is enough to check if only the leftmost index
field is not an expression (and references the remote table column)
and this check has already been done by
RemoteRelContainsLeftMostColumnOnIdx().

This commit removes IsIndexOnlyExpression() and
RemoteRelContainsLeftMostColumnOnIdx() and all checks for usable
indexes for REPLICA IDENTITY FULL tables are now performed by
IsIndexUsableForReplicaIdentityFull().

Backpatch this to remain the code consistent.

Reported-by: Peter Smith
Reviewed-by: Amit Kapila, Önder Kalacı
Discussion: https://postgr.es/m/CAHut%2BPsGRE5WSsY0jcLHJEoA17MrbP9yy8FxdjC_ZOAACxbt%2BQ%40mail.gmail.com
Backpatch-through: 16
---
 src/backend/executor/execReplication.c     |   9 --
 src/backend/replication/logical/relation.c | 118 ++++++++-------------
 src/backend/replication/logical/worker.c   |  24 ++++-
 src/include/replication/logicalrelation.h  |   2 +-
 4 files changed, 64 insertions(+), 89 deletions(-)

diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c
index e776524227..81f27042bc 100644
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -222,16 +222,7 @@ retry:
 		if (!isIdxSafeToSkipDuplicates)
 		{
 			if (eq == NULL)
-			{
-#ifdef USE_ASSERT_CHECKING
-				/* apply assertions only once for the input idxoid */
-				IndexInfo  *indexInfo = BuildIndexInfo(idxrel);
-
-				Assert(IsIndexUsableForReplicaIdentityFull(indexInfo));
-#endif
-
 				eq = palloc0(sizeof(*eq) * outslot->tts_tupleDescriptor->natts);
-			}
 
 			if (!tuples_equal(outslot, searchslot, eq))
 				continue;
diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index ed57e5d2b6..d62eefed13 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -734,71 +734,9 @@ logicalrep_partition_open(LogicalRepRelMapEntry *root,
 	return entry;
 }
 
-/*
- * Returns true if the given index consists only of expressions such as:
- * 	CREATE INDEX idx ON table(foo(col));
- *
- * Returns false even if there is one column reference:
- * 	 CREATE INDEX idx ON table(foo(col), col_2);
- */
-static bool
-IsIndexOnlyOnExpression(IndexInfo *indexInfo)
-{
-	for (int i = 0; i < indexInfo->ii_NumIndexKeyAttrs; i++)
-	{
-		AttrNumber	attnum = indexInfo->ii_IndexAttrNumbers[i];
-
-		if (AttributeNumberIsValid(attnum))
-			return false;
-	}
-
-	return true;
-}
-
-/*
- * Returns true if the attrmap contains the leftmost column of the index.
- * Otherwise returns false.
- *
- * attrmap is a map of local attributes to remote ones. We can consult this
- * map to check whether the local index attribute has a corresponding remote
- * attribute.
- */
-static bool
-RemoteRelContainsLeftMostColumnOnIdx(IndexInfo *indexInfo, AttrMap *attrmap)
-{
-	AttrNumber	keycol;
-
-	Assert(indexInfo->ii_NumIndexAttrs >= 1);
-
-	keycol = indexInfo->ii_IndexAttrNumbers[0];
-	if (!AttributeNumberIsValid(keycol))
-		return false;
-
-	if (attrmap->maplen <= AttrNumberGetAttrOffset(keycol))
-		return false;
-
-	return attrmap->attnums[AttrNumberGetAttrOffset(keycol)] >= 0;
-}
-
 /*
  * Returns the oid of an index that can be used by the apply worker to scan
- * the relation. The index must be btree or hash, non-partial, and the leftmost
- * field must be a column (not an expression) that references the remote
- * relation column. These limitations help to keep the index scan similar
- * to PK/RI index scans.
- *
- * Note that the limitations of index scans for replica identity full only
- * adheres to a subset of the limitations of PK/RI. For example, we support
- * columns that are marked as [NULL] or we are not interested in the [NOT
- * DEFERRABLE] aspect of constraints here. It works for us because we always
- * compare the tuples for non-PK/RI index scans. See
- * RelationFindReplTupleByIndex().
- *
- * XXX: See IsIndexUsableForReplicaIdentityFull() to know the challenges in
- * supporting indexes other than btree and hash. For partial indexes, the
- * required changes are likely to be larger. If none of the tuples satisfy
- * the expression for the index scan, we fall-back to sequential execution,
- * which might not be a good idea in some cases.
+ * the relation.
  *
  * We expect to call this function when REPLICA IDENTITY FULL is defined for
  * the remote relation.
@@ -815,19 +753,16 @@ FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap)
 	{
 		Oid			idxoid = lfirst_oid(lc);
 		bool		isUsableIdx;
-		bool		containsLeftMostCol;
 		Relation	idxRel;
 		IndexInfo  *idxInfo;
 
 		idxRel = index_open(idxoid, AccessShareLock);
 		idxInfo = BuildIndexInfo(idxRel);
-		isUsableIdx = IsIndexUsableForReplicaIdentityFull(idxInfo);
-		containsLeftMostCol =
-			RemoteRelContainsLeftMostColumnOnIdx(idxInfo, attrmap);
+		isUsableIdx = IsIndexUsableForReplicaIdentityFull(idxInfo, attrmap);
 		index_close(idxRel, AccessShareLock);
 
 		/* Return the first eligible index found */
-		if (isUsableIdx && containsLeftMostCol)
+		if (isUsableIdx)
 			return idxoid;
 	}
 
@@ -835,11 +770,24 @@ FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap)
 }
 
 /*
- * Returns true if the index is usable for replica identity full. For details,
- * see FindUsableIndexForReplicaIdentityFull.
+ * Returns true if the index is usable for replica identity full.
  *
- * Currently, only Btree and Hash indexes can be returned as usable. This
- * is due to following reasons:
+ * The index must be btree or hash, non-partial, and the leftmost field must be
+ * a column (not an expression) that references the remote relation column. These
+ * limitations help to keep the index scan similar to PK/RI index scans.
+ *
+ * attrmap is a map of local attributes to remote ones. We can consult this
+ * map to check whether the local index attribute has a corresponding remote
+ * attribute.
+ *
+ * Note that the limitations of index scans for replica identity full only
+ * adheres to a subset of the limitations of PK/RI. For example, we support
+ * columns that are marked as [NULL] or we are not interested in the [NOT
+ * DEFERRABLE] aspect of constraints here. It works for us because we always
+ * compare the tuples for non-PK/RI index scans. See
+ * RelationFindReplTupleByIndex().
+ *
+ * The reasons why only Btree and Hash indexes can be considered as usable are:
  *
  * 1) Other index access methods don't have a fixed strategy for equality
  * operation. Refer get_equal_strategy_number_for_am().
@@ -851,16 +799,38 @@ FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap)
  *
  * XXX: Note that BRIN and GIN indexes do not implement "amgettuple" which
  * will be used later to fetch the tuples. See RelationFindReplTupleByIndex().
+ *
+ * XXX: To support partial indexes, the required changes are likely to be larger.
+ * If none of the tuples satisfy the expression for the index scan, we fall-back
+ * to sequential execution, which might not be a good idea in some cases.
  */
 bool
-IsIndexUsableForReplicaIdentityFull(IndexInfo *indexInfo)
+IsIndexUsableForReplicaIdentityFull(IndexInfo *indexInfo, AttrMap *attrmap)
 {
+	AttrNumber	keycol;
+
 	/* Ensure that the index access method has a valid equal strategy */
 	if (get_equal_strategy_number_for_am(indexInfo->ii_Am) == InvalidStrategy)
 		return false;
+
+	/* The index must not be a partial index */
 	if (indexInfo->ii_Predicate != NIL)
 		return false;
-	if (IsIndexOnlyOnExpression(indexInfo))
+
+	Assert(indexInfo->ii_NumIndexAttrs >= 1);
+
+	/* The leftmost index field must not be an expression */
+	keycol = indexInfo->ii_IndexAttrNumbers[0];
+	if (!AttributeNumberIsValid(keycol))
+		return false;
+
+	/*
+	 * And the leftmost index field must reference the remote relation column.
+	 * This is because if it doesn't, the sequential scan is favorable over
+	 * index scan in most cases.
+	 */
+	if (attrmap->maplen <= AttrNumberGetAttrOffset(keycol) ||
+		attrmap->attnums[AttrNumberGetAttrOffset(keycol)] < 0)
 		return false;
 
 #ifdef USE_ASSERT_CHECKING
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index dd353fd1cb..0dd69739fb 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -140,6 +140,7 @@
 #include <sys/stat.h>
 #include <unistd.h>
 
+#include "access/genam.h"
 #include "access/table.h"
 #include "access/tableam.h"
 #include "access/twophase.h"
@@ -410,7 +411,7 @@ static void apply_handle_delete_internal(ApplyExecutionData *edata,
 										 ResultRelInfo *relinfo,
 										 TupleTableSlot *remoteslot,
 										 Oid localindexoid);
-static bool FindReplTupleInLocalRel(EState *estate, Relation localrel,
+static bool FindReplTupleInLocalRel(ApplyExecutionData *edata, Relation localrel,
 									LogicalRepRelation *remoterel,
 									Oid localidxoid,
 									TupleTableSlot *remoteslot,
@@ -2663,7 +2664,7 @@ apply_handle_update_internal(ApplyExecutionData *edata,
 	EvalPlanQualInit(&epqstate, estate, NULL, NIL, -1, NIL);
 	ExecOpenIndices(relinfo, false);
 
-	found = FindReplTupleInLocalRel(estate, localrel,
+	found = FindReplTupleInLocalRel(edata, localrel,
 									&relmapentry->remoterel,
 									localindexoid,
 									remoteslot, &localslot);
@@ -2816,7 +2817,7 @@ apply_handle_delete_internal(ApplyExecutionData *edata,
 	EvalPlanQualInit(&epqstate, estate, NULL, NIL, -1, NIL);
 	ExecOpenIndices(relinfo, false);
 
-	found = FindReplTupleInLocalRel(estate, localrel, remoterel, localindexoid,
+	found = FindReplTupleInLocalRel(edata, localrel, remoterel, localindexoid,
 									remoteslot, &localslot);
 
 	/* If found delete it. */
@@ -2855,12 +2856,13 @@ apply_handle_delete_internal(ApplyExecutionData *edata,
  * Local tuple, if found, is returned in '*localslot'.
  */
 static bool
-FindReplTupleInLocalRel(EState *estate, Relation localrel,
+FindReplTupleInLocalRel(ApplyExecutionData *edata, Relation localrel,
 						LogicalRepRelation *remoterel,
 						Oid localidxoid,
 						TupleTableSlot *remoteslot,
 						TupleTableSlot **localslot)
 {
+	EState	   *estate = edata->estate;
 	bool		found;
 
 	/*
@@ -2875,9 +2877,21 @@ FindReplTupleInLocalRel(EState *estate, Relation localrel,
 		   (remoterel->replident == REPLICA_IDENTITY_FULL));
 
 	if (OidIsValid(localidxoid))
+	{
+#ifdef USE_ASSERT_CHECKING
+		Relation	idxrel = index_open(localidxoid, AccessShareLock);
+
+		/* Index must be PK, RI, or usable for REPLICA IDENTITY FULL tables */
+		Assert(GetRelationIdentityOrPK(idxrel) == localidxoid ||
+			   IsIndexUsableForReplicaIdentityFull(BuildIndexInfo(idxrel),
+												   edata->targetRel->attrmap));
+		index_close(idxrel, AccessShareLock);
+#endif
+
 		found = RelationFindReplTupleByIndex(localrel, localidxoid,
 											 LockTupleExclusive,
 											 remoteslot, *localslot);
+	}
 	else
 		found = RelationFindReplTupleSeq(localrel, LockTupleExclusive,
 										 remoteslot, *localslot);
@@ -2995,7 +3009,7 @@ apply_handle_tuple_routing(ApplyExecutionData *edata,
 				bool		found;
 
 				/* Get the matching local tuple from the partition. */
-				found = FindReplTupleInLocalRel(estate, partrel,
+				found = FindReplTupleInLocalRel(edata, partrel,
 												&part_entry->remoterel,
 												part_entry->localindexoid,
 												remoteslot_part, &localslot);
diff --git a/src/include/replication/logicalrelation.h b/src/include/replication/logicalrelation.h
index 921b9974db..3f4d906d74 100644
--- a/src/include/replication/logicalrelation.h
+++ b/src/include/replication/logicalrelation.h
@@ -48,7 +48,7 @@ extern LogicalRepRelMapEntry *logicalrep_partition_open(LogicalRepRelMapEntry *r
 														Relation partrel, AttrMap *map);
 extern void logicalrep_rel_close(LogicalRepRelMapEntry *rel,
 								 LOCKMODE lockmode);
-extern bool IsIndexUsableForReplicaIdentityFull(IndexInfo *indexInfo);
+extern bool IsIndexUsableForReplicaIdentityFull(IndexInfo *indexInfo, AttrMap *attrmap);
 extern Oid	GetRelationIdentityOrPK(Relation rel);
 
 #endif							/* LOGICALRELATION_H */
-- 
2.31.1