public inbox for [email protected]  
help / color / mirror / Atom feed
From: Peter Eisentraut <[email protected]>
To: Paul Jungwirth <[email protected]>
Cc: jian he <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: SQL:2011 application time
Date: Tue, 26 Nov 2024 13:18:07 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <CA+renyUApHgSZF9-nd-a0+OPGharLQLO=mDHcY4_qQ0+noCUVg@mail.gmail.com>
	<[email protected]>
	<CACJufxESqArbu=R8m3a+ZF-Yia26KTGpEnyqz+8d5BXb1o=c=w@mail.gmail.com>
	<CACJufxECGaLjMcN8TJbX1fGrMFKs6qu+6RiZm98=sLcpiVpu2g@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CA+renyUFC13F0tYKxEENZtWA0YVuS5Tv+ZQkEkAwuDO1-Xke-A@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>

On 22.11.24 01:30, Paul Jungwirth wrote:
>> -       return get_equal_strategy_number_for_am(am);
>> +       /* For GiST indexes we need to ask the opclass what strategy 
>> number to use. */
>> +       if (am == GIST_AM_OID)
>> +               return GistTranslateStratnum(opclass, 
>> RTEqualStrategyNumber);
>> +       else
>> +               return get_equal_strategy_number_for_am(am);
>>
>> This code should probably be pushed into 
>> get_equal_strategy_number_for_am().  That function already
>> has a switch based on index AM OIDs.  Also, there are other callers of
>> get_equal_strategy_number_for_am(), which also might want to become 
>> aware of GiST support.
> 
> Done. The reason I didn't do this before is because we need the opclass, 
> not just the am. I put an
> explanation about that into the function comment. If that's a problem I 
> can undo the change.

I think this is the right idea, but after digging around a bit more, I 
think more could/should be done.

After these changes, the difference between 
get_equal_strategy_number_for_am() and get_equal_strategy_number() is 
kind of pointless.  We should really just use 
get_equal_strategy_number() for all purposes.

But then you have the problem that IsIndexUsableForReplicaIdentityFull() 
doesn't have the opclass IDs available in the IndexInfo structure.  You 
appear to have worked around that by writing

+   if (get_equal_strategy_number_for_am(indexInfo->ii_Am, InvalidOid) 
== InvalidStrategy)

which I suppose will have the same ultimate result as before that patch, 
but it seems kind of incomplete.

I figure this could all be simpler if 
IsIndexUsableForReplicaIdentityFull() used the index relcache entry 
directly instead of going the detour through IndexInfo.  Then we have 
all the information available, and this should ultimately all work 
properly for suitable GiST indexes as well.

I have attached three patches that show how that could be done.  (This 
would work in conjunction with your new tests.  (Although now we could 
also test GiST with replica identity full?))

The comment block for IsIndexUsableForReplicaIdentityFull() makes a 
bunch of claims that are not all explicitly supported by the code.  The 
code doesn't actually check the AM, this is all only done indirectly via 
other checks.  The second point (about tuples_equal()) appears to be 
slightly wrong, because while you need an equals operator from the type 
cache, that shouldn't prevent you from also using a different index AM 
than btree or hash for the replica identity index.  And the stuff about 
amgettuple, if that is important, why is it only checked for assert builds?

From fdf5a2335cb987953b070dec19e20a3f9caaaa5e Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Tue, 26 Nov 2024 11:25:12 +0100
Subject: [PATCH 1/3] Simplify IsIndexUsableForReplicaIdentityFull()

Take Relation as argument instead of IndexInfo.  Building the
IndexInfo is an unnecessary intermediate step here.
---
 src/backend/replication/logical/relation.c | 16 +++++++---------
 src/backend/replication/logical/worker.c   |  2 +-
 src/include/replication/logicalrelation.h  |  2 +-
 3 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index f5a0ef2bd9d..1b426f43fd7 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -750,11 +750,9 @@ FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap)
 	{
 		bool		isUsableIdx;
 		Relation	idxRel;
-		IndexInfo  *idxInfo;
 
 		idxRel = index_open(idxoid, AccessShareLock);
-		idxInfo = BuildIndexInfo(idxRel);
-		isUsableIdx = IsIndexUsableForReplicaIdentityFull(idxInfo, attrmap);
+		isUsableIdx = IsIndexUsableForReplicaIdentityFull(idxRel, attrmap);
 		index_close(idxRel, AccessShareLock);
 
 		/* Return the first eligible index found */
@@ -801,22 +799,22 @@ FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap)
  * to sequential execution, which might not be a good idea in some cases.
  */
 bool
-IsIndexUsableForReplicaIdentityFull(IndexInfo *indexInfo, AttrMap *attrmap)
+IsIndexUsableForReplicaIdentityFull(Relation idxrel, 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)
+	if (get_equal_strategy_number_for_am(idxrel->rd_rel->relam) == InvalidStrategy)
 		return false;
 
 	/* The index must not be a partial index */
-	if (indexInfo->ii_Predicate != NIL)
+	if (!heap_attisnull(idxrel->rd_indextuple, Anum_pg_index_indpred, NULL))
 		return false;
 
-	Assert(indexInfo->ii_NumIndexAttrs >= 1);
+	Assert(idxrel->rd_index->indnatts >= 1);
 
 	/* The leftmost index field must not be an expression */
-	keycol = indexInfo->ii_IndexAttrNumbers[0];
+	keycol = idxrel->rd_index->indkey.values[0];
 	if (!AttributeNumberIsValid(keycol))
 		return false;
 
@@ -834,7 +832,7 @@ IsIndexUsableForReplicaIdentityFull(IndexInfo *indexInfo, AttrMap *attrmap)
 		IndexAmRoutine *amroutine;
 
 		/* The given index access method must implement amgettuple. */
-		amroutine = GetIndexAmRoutineByAmId(indexInfo->ii_Am, false);
+		amroutine = GetIndexAmRoutineByAmId(idxrel->rd_rel->relam, false);
 		Assert(amroutine->amgettuple != NULL);
 	}
 #endif
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 925dff9cc44..46d3ad566f6 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -2931,7 +2931,7 @@ FindReplTupleInLocalRel(ApplyExecutionData *edata, Relation localrel,
 		/* Index must be PK, RI, or usable for REPLICA IDENTITY FULL tables */
 		Assert(GetRelationIdentityOrPK(localrel) == localidxoid ||
 			   (remoterel->replident == REPLICA_IDENTITY_FULL &&
-				IsIndexUsableForReplicaIdentityFull(BuildIndexInfo(idxrel),
+				IsIndexUsableForReplicaIdentityFull(idxrel,
 													edata->targetRel->attrmap)));
 		index_close(idxrel, AccessShareLock);
 #endif
diff --git a/src/include/replication/logicalrelation.h b/src/include/replication/logicalrelation.h
index e687b40a566..33534672ec3 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, AttrMap *attrmap);
+extern bool IsIndexUsableForReplicaIdentityFull(Relation idxrel, AttrMap *attrmap);
 extern Oid	GetRelationIdentityOrPK(Relation rel);
 
 #endif							/* LOGICALRELATION_H */
-- 
2.47.0


From e24695c4a912eeefe0cfc2fbfa5369a0374e6223 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Tue, 26 Nov 2024 11:47:24 +0100
Subject: [PATCH 2/3] Replace get_equal_strategy_number_for_am() by
 get_equal_strategy_number()

---
 src/backend/executor/execReplication.c     | 17 +++--------------
 src/backend/replication/logical/relation.c | 15 +++++++++++----
 src/include/executor/executor.h            |  2 +-
 3 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c
index 54025c9f150..8313ccdb46b 100644
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -39,7 +39,7 @@ static bool tuples_equal(TupleTableSlot *slot1, TupleTableSlot *slot2,
 
 /*
  * Returns the fixed strategy number, if any, of the equality operator for the
- * given index access method, otherwise, InvalidStrategy.
+ * given operator class, otherwise, InvalidStrategy.
  *
  * Currently, only Btree and Hash indexes are supported. The other index access
  * methods don't have a fixed strategy for equality operation - instead, the
@@ -47,8 +47,9 @@ static bool tuples_equal(TupleTableSlot *slot1, TupleTableSlot *slot2,
  * according to the operator class's definition.
  */
 StrategyNumber
-get_equal_strategy_number_for_am(Oid am)
+get_equal_strategy_number(Oid opclass)
 {
+	Oid			am = get_opclass_method(opclass);
 	int			ret;
 
 	switch (am)
@@ -68,18 +69,6 @@ get_equal_strategy_number_for_am(Oid am)
 	return ret;
 }
 
-/*
- * Return the appropriate strategy number which corresponds to the equality
- * operator.
- */
-static StrategyNumber
-get_equal_strategy_number(Oid opclass)
-{
-	Oid			am = get_opclass_method(opclass);
-
-	return get_equal_strategy_number_for_am(am);
-}
-
 /*
  * Setup a ScanKey for a search in the relation 'rel' for a tuple 'key' that
  * is setup to match 'rel' (*NOT* idxrel!).
diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index 1b426f43fd7..7cda6a792a2 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -29,6 +29,7 @@
 #include "replication/logicalrelation.h"
 #include "replication/worker_internal.h"
 #include "utils/inval.h"
+#include "utils/syscache.h"
 
 
 static MemoryContext LogicalRepRelMapContext = NULL;
@@ -784,7 +785,7 @@ FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap)
  * 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().
+ * operation. Refer get_equal_strategy_number().
  *
  * 2) For indexes other than PK and REPLICA IDENTITY, we need to match the
  * local and remote tuples. The equality routine tuples_equal() cannot accept
@@ -802,10 +803,9 @@ bool
 IsIndexUsableForReplicaIdentityFull(Relation idxrel, AttrMap *attrmap)
 {
 	AttrNumber	keycol;
+	oidvector  *indclass;
 
-	/* Ensure that the index access method has a valid equal strategy */
-	if (get_equal_strategy_number_for_am(idxrel->rd_rel->relam) == InvalidStrategy)
-		return false;
+	indclass = (oidvector *) DatumGetPointer(SysCacheGetAttrNotNull(INDEXRELID, idxrel->rd_indextuple, Anum_pg_index_indclass));
 
 	/* The index must not be a partial index */
 	if (!heap_attisnull(idxrel->rd_indextuple, Anum_pg_index_indpred, NULL))
@@ -813,6 +813,13 @@ IsIndexUsableForReplicaIdentityFull(Relation idxrel, AttrMap *attrmap)
 
 	Assert(idxrel->rd_index->indnatts >= 1);
 
+	/* Ensure that the index has a valid equal strategy for each column */
+	for (int i = 0; i < idxrel->rd_index->indnatts; i++)
+	{
+		if (get_equal_strategy_number(indclass->values[i]) == InvalidStrategy)
+			return false;
+	}
+
 	/* The leftmost index field must not be an expression */
 	keycol = idxrel->rd_index->indkey.values[0];
 	if (!AttributeNumberIsValid(keycol))
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index 69c3ebff00a..e949cce7d98 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -658,7 +658,7 @@ extern void check_exclusion_constraint(Relation heap, Relation index,
 /*
  * prototypes from functions in execReplication.c
  */
-extern StrategyNumber get_equal_strategy_number_for_am(Oid am);
+extern StrategyNumber get_equal_strategy_number(Oid opclass);
 extern bool RelationFindReplTupleByIndex(Relation rel, Oid idxoid,
 										 LockTupleMode lockmode,
 										 TupleTableSlot *searchslot,
-- 
2.47.0


From a46139648ac7c4de927315158b66c15bd2fd5360 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Tue, 26 Nov 2024 11:57:08 +0100
Subject: [PATCH 3/3] Support for GIST in get_equal_strategy_number()

---
 src/backend/executor/execReplication.c     | 9 ++++-----
 src/backend/replication/logical/relation.c | 2 +-
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c
index 8313ccdb46b..0fa67adb990 100644
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -15,6 +15,7 @@
 #include "postgres.h"
 
 #include "access/genam.h"
+#include "access/gist.h"
 #include "access/relscan.h"
 #include "access/tableam.h"
 #include "access/transam.h"
@@ -40,11 +41,6 @@ static bool tuples_equal(TupleTableSlot *slot1, TupleTableSlot *slot2,
 /*
  * Returns the fixed strategy number, if any, of the equality operator for the
  * given operator class, otherwise, InvalidStrategy.
- *
- * Currently, only Btree and Hash indexes are supported. The other index access
- * methods don't have a fixed strategy for equality operation - instead, the
- * support routines of each operator class interpret the strategy numbers
- * according to the operator class's definition.
  */
 StrategyNumber
 get_equal_strategy_number(Oid opclass)
@@ -60,6 +56,9 @@ get_equal_strategy_number(Oid opclass)
 		case HASH_AM_OID:
 			ret = HTEqualStrategyNumber;
 			break;
+		case GIST_AM_OID:
+			ret = GistTranslateStratnum(opclass, RTEqualStrategyNumber);
+			break;
 		default:
 			/* XXX: Only Btree and Hash indexes are supported */
 			ret = InvalidStrategy;
diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index 7cda6a792a2..f686975f39d 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -782,7 +782,7 @@ FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap)
  * 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:
+ * The reasons why only Btree and Hash indexes can be considered as usable are: XXX
  *
  * 1) Other index access methods don't have a fixed strategy for equality
  * operation. Refer get_equal_strategy_number().
-- 
2.47.0



Attachments:

  [text/plain] 0001-Simplify-IsIndexUsableForReplicaIdentityFull.patch.nocfbot (4.1K, ../[email protected]/2-0001-Simplify-IsIndexUsableForReplicaIdentityFull.patch.nocfbot)
  download | inline diff:
From fdf5a2335cb987953b070dec19e20a3f9caaaa5e Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Tue, 26 Nov 2024 11:25:12 +0100
Subject: [PATCH 1/3] Simplify IsIndexUsableForReplicaIdentityFull()

Take Relation as argument instead of IndexInfo.  Building the
IndexInfo is an unnecessary intermediate step here.
---
 src/backend/replication/logical/relation.c | 16 +++++++---------
 src/backend/replication/logical/worker.c   |  2 +-
 src/include/replication/logicalrelation.h  |  2 +-
 3 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index f5a0ef2bd9d..1b426f43fd7 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -750,11 +750,9 @@ FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap)
 	{
 		bool		isUsableIdx;
 		Relation	idxRel;
-		IndexInfo  *idxInfo;
 
 		idxRel = index_open(idxoid, AccessShareLock);
-		idxInfo = BuildIndexInfo(idxRel);
-		isUsableIdx = IsIndexUsableForReplicaIdentityFull(idxInfo, attrmap);
+		isUsableIdx = IsIndexUsableForReplicaIdentityFull(idxRel, attrmap);
 		index_close(idxRel, AccessShareLock);
 
 		/* Return the first eligible index found */
@@ -801,22 +799,22 @@ FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap)
  * to sequential execution, which might not be a good idea in some cases.
  */
 bool
-IsIndexUsableForReplicaIdentityFull(IndexInfo *indexInfo, AttrMap *attrmap)
+IsIndexUsableForReplicaIdentityFull(Relation idxrel, 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)
+	if (get_equal_strategy_number_for_am(idxrel->rd_rel->relam) == InvalidStrategy)
 		return false;
 
 	/* The index must not be a partial index */
-	if (indexInfo->ii_Predicate != NIL)
+	if (!heap_attisnull(idxrel->rd_indextuple, Anum_pg_index_indpred, NULL))
 		return false;
 
-	Assert(indexInfo->ii_NumIndexAttrs >= 1);
+	Assert(idxrel->rd_index->indnatts >= 1);
 
 	/* The leftmost index field must not be an expression */
-	keycol = indexInfo->ii_IndexAttrNumbers[0];
+	keycol = idxrel->rd_index->indkey.values[0];
 	if (!AttributeNumberIsValid(keycol))
 		return false;
 
@@ -834,7 +832,7 @@ IsIndexUsableForReplicaIdentityFull(IndexInfo *indexInfo, AttrMap *attrmap)
 		IndexAmRoutine *amroutine;
 
 		/* The given index access method must implement amgettuple. */
-		amroutine = GetIndexAmRoutineByAmId(indexInfo->ii_Am, false);
+		amroutine = GetIndexAmRoutineByAmId(idxrel->rd_rel->relam, false);
 		Assert(amroutine->amgettuple != NULL);
 	}
 #endif
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 925dff9cc44..46d3ad566f6 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -2931,7 +2931,7 @@ FindReplTupleInLocalRel(ApplyExecutionData *edata, Relation localrel,
 		/* Index must be PK, RI, or usable for REPLICA IDENTITY FULL tables */
 		Assert(GetRelationIdentityOrPK(localrel) == localidxoid ||
 			   (remoterel->replident == REPLICA_IDENTITY_FULL &&
-				IsIndexUsableForReplicaIdentityFull(BuildIndexInfo(idxrel),
+				IsIndexUsableForReplicaIdentityFull(idxrel,
 													edata->targetRel->attrmap)));
 		index_close(idxrel, AccessShareLock);
 #endif
diff --git a/src/include/replication/logicalrelation.h b/src/include/replication/logicalrelation.h
index e687b40a566..33534672ec3 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, AttrMap *attrmap);
+extern bool IsIndexUsableForReplicaIdentityFull(Relation idxrel, AttrMap *attrmap);
 extern Oid	GetRelationIdentityOrPK(Relation rel);
 
 #endif							/* LOGICALRELATION_H */
-- 
2.47.0



  [text/plain] 0002-Replace-get_equal_strategy_number_for_am-by-.patch.nocfbot (4.5K, ../[email protected]/3-0002-Replace-get_equal_strategy_number_for_am-by-.patch.nocfbot)
  download | inline diff:
From e24695c4a912eeefe0cfc2fbfa5369a0374e6223 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Tue, 26 Nov 2024 11:47:24 +0100
Subject: [PATCH 2/3] Replace get_equal_strategy_number_for_am() by
 get_equal_strategy_number()

---
 src/backend/executor/execReplication.c     | 17 +++--------------
 src/backend/replication/logical/relation.c | 15 +++++++++++----
 src/include/executor/executor.h            |  2 +-
 3 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c
index 54025c9f150..8313ccdb46b 100644
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -39,7 +39,7 @@ static bool tuples_equal(TupleTableSlot *slot1, TupleTableSlot *slot2,
 
 /*
  * Returns the fixed strategy number, if any, of the equality operator for the
- * given index access method, otherwise, InvalidStrategy.
+ * given operator class, otherwise, InvalidStrategy.
  *
  * Currently, only Btree and Hash indexes are supported. The other index access
  * methods don't have a fixed strategy for equality operation - instead, the
@@ -47,8 +47,9 @@ static bool tuples_equal(TupleTableSlot *slot1, TupleTableSlot *slot2,
  * according to the operator class's definition.
  */
 StrategyNumber
-get_equal_strategy_number_for_am(Oid am)
+get_equal_strategy_number(Oid opclass)
 {
+	Oid			am = get_opclass_method(opclass);
 	int			ret;
 
 	switch (am)
@@ -68,18 +69,6 @@ get_equal_strategy_number_for_am(Oid am)
 	return ret;
 }
 
-/*
- * Return the appropriate strategy number which corresponds to the equality
- * operator.
- */
-static StrategyNumber
-get_equal_strategy_number(Oid opclass)
-{
-	Oid			am = get_opclass_method(opclass);
-
-	return get_equal_strategy_number_for_am(am);
-}
-
 /*
  * Setup a ScanKey for a search in the relation 'rel' for a tuple 'key' that
  * is setup to match 'rel' (*NOT* idxrel!).
diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index 1b426f43fd7..7cda6a792a2 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -29,6 +29,7 @@
 #include "replication/logicalrelation.h"
 #include "replication/worker_internal.h"
 #include "utils/inval.h"
+#include "utils/syscache.h"
 
 
 static MemoryContext LogicalRepRelMapContext = NULL;
@@ -784,7 +785,7 @@ FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap)
  * 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().
+ * operation. Refer get_equal_strategy_number().
  *
  * 2) For indexes other than PK and REPLICA IDENTITY, we need to match the
  * local and remote tuples. The equality routine tuples_equal() cannot accept
@@ -802,10 +803,9 @@ bool
 IsIndexUsableForReplicaIdentityFull(Relation idxrel, AttrMap *attrmap)
 {
 	AttrNumber	keycol;
+	oidvector  *indclass;
 
-	/* Ensure that the index access method has a valid equal strategy */
-	if (get_equal_strategy_number_for_am(idxrel->rd_rel->relam) == InvalidStrategy)
-		return false;
+	indclass = (oidvector *) DatumGetPointer(SysCacheGetAttrNotNull(INDEXRELID, idxrel->rd_indextuple, Anum_pg_index_indclass));
 
 	/* The index must not be a partial index */
 	if (!heap_attisnull(idxrel->rd_indextuple, Anum_pg_index_indpred, NULL))
@@ -813,6 +813,13 @@ IsIndexUsableForReplicaIdentityFull(Relation idxrel, AttrMap *attrmap)
 
 	Assert(idxrel->rd_index->indnatts >= 1);
 
+	/* Ensure that the index has a valid equal strategy for each column */
+	for (int i = 0; i < idxrel->rd_index->indnatts; i++)
+	{
+		if (get_equal_strategy_number(indclass->values[i]) == InvalidStrategy)
+			return false;
+	}
+
 	/* The leftmost index field must not be an expression */
 	keycol = idxrel->rd_index->indkey.values[0];
 	if (!AttributeNumberIsValid(keycol))
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index 69c3ebff00a..e949cce7d98 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -658,7 +658,7 @@ extern void check_exclusion_constraint(Relation heap, Relation index,
 /*
  * prototypes from functions in execReplication.c
  */
-extern StrategyNumber get_equal_strategy_number_for_am(Oid am);
+extern StrategyNumber get_equal_strategy_number(Oid opclass);
 extern bool RelationFindReplTupleByIndex(Relation rel, Oid idxoid,
 										 LockTupleMode lockmode,
 										 TupleTableSlot *searchslot,
-- 
2.47.0



  [text/plain] 0003-Support-for-GIST-in-get_equal_strategy_numbe.patch.nocfbot (2.4K, ../[email protected]/4-0003-Support-for-GIST-in-get_equal_strategy_numbe.patch.nocfbot)
  download | inline diff:
From a46139648ac7c4de927315158b66c15bd2fd5360 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Tue, 26 Nov 2024 11:57:08 +0100
Subject: [PATCH 3/3] Support for GIST in get_equal_strategy_number()

---
 src/backend/executor/execReplication.c     | 9 ++++-----
 src/backend/replication/logical/relation.c | 2 +-
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c
index 8313ccdb46b..0fa67adb990 100644
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -15,6 +15,7 @@
 #include "postgres.h"
 
 #include "access/genam.h"
+#include "access/gist.h"
 #include "access/relscan.h"
 #include "access/tableam.h"
 #include "access/transam.h"
@@ -40,11 +41,6 @@ static bool tuples_equal(TupleTableSlot *slot1, TupleTableSlot *slot2,
 /*
  * Returns the fixed strategy number, if any, of the equality operator for the
  * given operator class, otherwise, InvalidStrategy.
- *
- * Currently, only Btree and Hash indexes are supported. The other index access
- * methods don't have a fixed strategy for equality operation - instead, the
- * support routines of each operator class interpret the strategy numbers
- * according to the operator class's definition.
  */
 StrategyNumber
 get_equal_strategy_number(Oid opclass)
@@ -60,6 +56,9 @@ get_equal_strategy_number(Oid opclass)
 		case HASH_AM_OID:
 			ret = HTEqualStrategyNumber;
 			break;
+		case GIST_AM_OID:
+			ret = GistTranslateStratnum(opclass, RTEqualStrategyNumber);
+			break;
 		default:
 			/* XXX: Only Btree and Hash indexes are supported */
 			ret = InvalidStrategy;
diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index 7cda6a792a2..f686975f39d 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -782,7 +782,7 @@ FindUsableIndexForReplicaIdentityFull(Relation localrel, AttrMap *attrmap)
  * 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:
+ * The reasons why only Btree and Hash indexes can be considered as usable are: XXX
  *
  * 1) Other index access methods don't have a fixed strategy for equality
  * operation. Refer get_equal_strategy_number().
-- 
2.47.0



view thread (35+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: SQL:2011 application time
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox