v19.2-0006-Integrate-GistTranslateCompareType-into-IndexA.patch

text/plain

Filename: v19.2-0006-Integrate-GistTranslateCompareType-into-IndexA.patch
Type: text/plain
Part: 5
Message: Re: Index AM API cleanup

Patch

Same data as JSON: GET /api/v1/attachments/:id/patch the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes. API reference →
Format: format-patch
Series: patch v19-0006
Subject: Integrate GistTranslateCompareType() into IndexAmTranslateCompareType()
File+
src/backend/access/gist/gist.c 1 1
src/backend/access/gist/gistutil.c 1 7
src/backend/commands/indexcmds.c 6 16
src/backend/commands/tablecmds.c 6 22
src/backend/executor/execReplication.c 1 33
src/backend/replication/logical/relation.c 7 1
src/include/access/gist.h 1 1
src/include/executor/executor.h 0 1
From 23c0334e05f0bd4baf5b606264f9ff532adf116b Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Fri, 24 Jan 2025 16:55:47 +0100
Subject: [PATCH v19.2 6/6] Integrate GistTranslateCompareType() into
 IndexAmTranslateCompareType()

This turns GistTranslateCompareType() into a callback function of the
gist index AM instead of a standalone function.  The existing callers
are changed to use IndexAmTranslateCompareType().  This then also
makes that code not or less hardcoded toward gist.
---
 src/backend/access/gist/gist.c             |  2 +-
 src/backend/access/gist/gistutil.c         |  8 +----
 src/backend/commands/indexcmds.c           | 22 ++++----------
 src/backend/commands/tablecmds.c           | 28 ++++--------------
 src/backend/executor/execReplication.c     | 34 +---------------------
 src/backend/replication/logical/relation.c |  8 ++++-
 src/include/access/gist.h                  |  2 +-
 src/include/executor/executor.h            |  1 -
 8 files changed, 23 insertions(+), 82 deletions(-)

diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c
index 70f8086db7b..4d858b65e1e 100644
--- a/src/backend/access/gist/gist.c
+++ b/src/backend/access/gist/gist.c
@@ -108,7 +108,7 @@ gisthandler(PG_FUNCTION_ARGS)
 	amroutine->aminitparallelscan = NULL;
 	amroutine->amparallelrescan = NULL;
 	amroutine->amtranslatestrategy = NULL;
-	amroutine->amtranslatecmptype = NULL;
+	amroutine->amtranslatecmptype = gisttranslatecmptype;
 
 	PG_RETURN_POINTER(amroutine);
 }
diff --git a/src/backend/access/gist/gistutil.c b/src/backend/access/gist/gistutil.c
index 4d3b6dfa32b..dbc4ac639a2 100644
--- a/src/backend/access/gist/gistutil.c
+++ b/src/backend/access/gist/gistutil.c
@@ -1095,17 +1095,11 @@ gist_stratnum_common(PG_FUNCTION_ARGS)
  * Returns InvalidStrategy if the function is not defined.
  */
 StrategyNumber
-GistTranslateCompareType(Oid opclass, CompareType cmptype)
+gisttranslatecmptype(CompareType cmptype, Oid opfamily, Oid opcintype)
 {
-	Oid			opfamily;
-	Oid			opcintype;
 	Oid			funcid;
 	Datum		result;
 
-	/* Look up the opclass family and input datatype. */
-	if (!get_opclass_opfamily_and_input_type(opclass, &opfamily, &opcintype))
-		return InvalidStrategy;
-
 	/* Check whether the function is provided. */
 	funcid = get_opfamily_proc(opfamily, opcintype, opcintype, GIST_STRATNUM_PROC);
 	if (!OidIsValid(funcid))
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index f788b8f29b8..637ff52b505 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2422,6 +2422,7 @@ void
 GetOperatorFromCompareType(Oid opclass, Oid rhstype, CompareType cmptype,
 						   Oid *opid, StrategyNumber *strat)
 {
+	Oid			amid = GIST_AM_OID;	/* For now we only need GiST support. */
 	Oid			opfamily;
 	Oid			opcintype;
 
@@ -2432,28 +2433,17 @@ GetOperatorFromCompareType(Oid opclass, Oid rhstype, CompareType cmptype,
 	if (get_opclass_opfamily_and_input_type(opclass, &opfamily, &opcintype))
 	{
 		/*
-		 * Ask the opclass to translate to its internal stratnum
-		 *
-		 * For now we only need GiST support, but this could support other
-		 * indexams if we wanted.
+		 * Ask the index AM to translate to its internal stratnum
 		 */
-		*strat = GistTranslateCompareType(opclass, cmptype);
+		*strat = IndexAmTranslateCompareType(cmptype, amid, opfamily, opcintype, true);
 		if (*strat == InvalidStrategy)
-		{
-			HeapTuple	tuple;
-
-			tuple = SearchSysCache1(CLAOID, ObjectIdGetDatum(opclass));
-			if (!HeapTupleIsValid(tuple))
-				elog(ERROR, "cache lookup failed for operator class %u", opclass);
-
 			ereport(ERROR,
 					errcode(ERRCODE_UNDEFINED_OBJECT),
 					cmptype = COMPARE_EQ ? errmsg("could not identify an equality operator for type %s", format_type_be(opcintype)) :
 					cmptype == COMPARE_OVERLAP ? errmsg("could not identify an overlaps operator for type %s", format_type_be(opcintype)) :
 					cmptype == COMPARE_CONTAINED_BY ? errmsg("could not identify a contained-by operator for type %s", format_type_be(opcintype)) : 0,
-					errdetail("Could not translate compare type %d for operator class \"%s\" for access method \"%s\".",
-							  cmptype, NameStr(((Form_pg_opclass) GETSTRUCT(tuple))->opcname), "gist"));
-		}
+					errdetail("Could not translate compare type %d for operator family \"%s\", input type %s, access method \"%s\".",
+							  cmptype, get_opfamily_name(opfamily, false), format_type_be(opcintype), get_am_name(amid)));
 
 		/*
 		 * We parameterize rhstype so foreign keys can ask for a <@ operator
@@ -2472,7 +2462,7 @@ GetOperatorFromCompareType(Oid opclass, Oid rhstype, CompareType cmptype,
 				cmptype == COMPARE_OVERLAP ? errmsg("could not identify an overlaps operator for type %s", format_type_be(opcintype)) :
 				cmptype == COMPARE_CONTAINED_BY ? errmsg("could not identify a contained-by operator for type %s", format_type_be(opcintype)) : 0,
 				errdetail("There is no suitable operator in operator family \"%s\" for access method \"%s\".",
-						  get_opfamily_name(opfamily, false), "gist"));
+						  get_opfamily_name(opfamily, false), get_am_name(amid)));
 }
 
 /*
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index d51298284f6..9e027f1b328 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -10002,37 +10002,21 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
 			CompareType cmptype;
 			bool		for_overlaps = with_period && i == numpks - 1;
 
-			/*
-			 * GiST indexes are required to support temporal foreign keys
-			 * because they combine equals and overlaps.
-			 */
-			if (amid != GIST_AM_OID)
-				elog(ERROR, "only GiST indexes are supported for temporal foreign keys");
-
 			cmptype = for_overlaps ? COMPARE_OVERLAP : COMPARE_EQ;
 
 			/*
-			 * An opclass can use whatever strategy numbers it wants, so we
-			 * ask the opclass what number it actually uses instead of our RT*
-			 * constants.
+			 * An index AM can use whatever strategy numbers it wants, so we
+			 * ask it what number it actually uses.
 			 */
-			eqstrategy = GistTranslateCompareType(opclasses[i], cmptype);
+			eqstrategy = IndexAmTranslateCompareType(cmptype, amid, opfamily, opcintype, true);
 			if (eqstrategy == InvalidStrategy)
-			{
-				HeapTuple	tuple;
-
-				tuple = SearchSysCache1(CLAOID, ObjectIdGetDatum(opclasses[i]));
-				if (!HeapTupleIsValid(tuple))
-					elog(ERROR, "cache lookup failed for operator class %u", opclasses[i]);
-
 				ereport(ERROR,
 						errcode(ERRCODE_UNDEFINED_OBJECT),
 						for_overlaps
 						? errmsg("could not identify an overlaps operator for foreign key")
 						: errmsg("could not identify an equality operator for foreign key"),
-						errdetail("Could not translate compare type %d for operator class \"%s\" for access method \"%s\".",
-								  cmptype, NameStr(((Form_pg_opclass) GETSTRUCT(tuple))->opcname), "gist"));
-			}
+						errdetail("Could not translate compare type %d for operator family \"%s\", input type %s, access method \"%s\".",
+								  cmptype, get_opfamily_name(opfamily, false), format_type_be(opcintype), get_am_name(amid)));
 		}
 		else
 		{
@@ -10041,7 +10025,7 @@ ATAddForeignKeyConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
 			 * other index AMs support unique indexes.  If we ever did have
 			 * other types of unique indexes, we'd need a way to determine
 			 * which operator strategy number is equality.  (We could use
-			 * something like GistTranslateCompareType.)
+			 * IndexAmTranslateCompareType.)
 			 */
 			if (amid != BTREE_AM_OID)
 				elog(ERROR, "only b-tree indexes are supported for foreign keys");
diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c
index 2dac4bd363b..5f7613cc831 100644
--- a/src/backend/executor/execReplication.c
+++ b/src/backend/executor/execReplication.c
@@ -38,35 +38,6 @@
 static bool tuples_equal(TupleTableSlot *slot1, TupleTableSlot *slot2,
 						 TypeCacheEntry **eq);
 
-/*
- * Returns the fixed strategy number, if any, of the equality operator for the
- * given operator class, otherwise, InvalidStrategy.
- */
-StrategyNumber
-get_equal_strategy_number(Oid opclass)
-{
-	Oid			am = get_opclass_method(opclass);
-	int			ret;
-
-	switch (am)
-	{
-		case BTREE_AM_OID:
-			ret = BTEqualStrategyNumber;
-			break;
-		case HASH_AM_OID:
-			ret = HTEqualStrategyNumber;
-			break;
-		case GIST_AM_OID:
-			ret = GistTranslateCompareType(opclass, COMPARE_EQ);
-			break;
-		default:
-			ret = InvalidStrategy;
-			break;
-	}
-
-	return ret;
-}
-
 /*
  * Setup a ScanKey for a search in the relation 'rel' for a tuple 'key' that
  * is setup to match 'rel' (*NOT* idxrel!).
@@ -120,10 +91,7 @@ build_replindex_scan_key(ScanKey skey, Relation rel, Relation idxrel,
 		 */
 		optype = get_opclass_input_type(opclass->values[index_attoff]);
 		opfamily = get_opclass_family(opclass->values[index_attoff]);
-		eq_strategy = get_equal_strategy_number(opclass->values[index_attoff]);
-		if (!eq_strategy)
-			elog(ERROR, "missing equal strategy for opclass %u", opclass->values[index_attoff]);
-
+		eq_strategy = IndexAmTranslateCompareType(COMPARE_EQ, idxrel->rd_rel->relam, opfamily, optype, false);
 		operator = get_opfamily_member(opfamily, optype,
 									   optype,
 									   eq_strategy);
diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index 67aab02ff76..e9ad90d64a5 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -27,6 +27,7 @@
 #include "replication/logicalrelation.h"
 #include "replication/worker_internal.h"
 #include "utils/inval.h"
+#include "utils/lsyscache.h"
 #include "utils/syscache.h"
 
 
@@ -835,7 +836,12 @@ IsIndexUsableForReplicaIdentityFull(Relation idxrel, AttrMap *attrmap)
 	/* Ensure that the index has a valid equal strategy for each key column */
 	for (int i = 0; i < idxrel->rd_index->indnkeyatts; i++)
 	{
-		if (get_equal_strategy_number(indclass->values[i]) == InvalidStrategy)
+		Oid			opfamily;
+		Oid			opcintype;
+
+		if (!get_opclass_opfamily_and_input_type(indclass->values[i], &opfamily, &opcintype))
+			return false;
+		if (IndexAmTranslateCompareType(COMPARE_EQ, idxrel->rd_rel->relam, opfamily, opcintype, true) == InvalidStrategy)
 			return false;
 	}
 
diff --git a/src/include/access/gist.h b/src/include/access/gist.h
index eff019f7515..db62a9ac0b4 100644
--- a/src/include/access/gist.h
+++ b/src/include/access/gist.h
@@ -248,6 +248,6 @@ typedef struct
 	do { (e).key = (k); (e).rel = (r); (e).page = (pg); \
 		 (e).offset = (o); (e).leafkey = (l); } while (0)
 
-extern StrategyNumber GistTranslateCompareType(Oid opclass, CompareType cmptype);
+extern StrategyNumber gisttranslatecmptype(CompareType cmptype, Oid opfamily, Oid opcintype);
 
 #endif							/* GIST_H */
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index c7db6defd3e..45b80e6b98e 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -664,7 +664,6 @@ extern void check_exclusion_constraint(Relation heap, Relation index,
 /*
  * prototypes from functions in execReplication.c
  */
-extern StrategyNumber get_equal_strategy_number(Oid opclass);
 extern bool RelationFindReplTupleByIndex(Relation rel, Oid idxoid,
 										 LockTupleMode lockmode,
 										 TupleTableSlot *searchslot,
-- 
2.48.1