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 makes
that code not hardcoded toward gist.

This means in particular that the temporal keys code is now
independent of gist.  Also, this generalizes commit 74edabce7a3, so
other index access methods other than the previously hardcoded ones
could now work as REPLICA IDENTITY in a logical replication
subscriber.

Author: Mark Dilger <mark.dilger@enterprisedb.com>
Co-authored-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://www.postgresql.org/message-id/flat/E72EAA49-354D-4C2E-8EB9-255197F55330@enterprisedb.com

diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c
index 70f8086db7bd415800e206ca4346b45f73a81d32..4d858b65e1e9455027c4d4823ebb61a3a6e588af 100644 (file)
--- 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 4d3b6dfa32b3be601930291e529b8258309ccd57..dbc4ac639a231681983050174948ed8e22f1a234 100644 (file)
--- 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 f788b8f29b85b0f46b03fc3a7007beeb170a2baa..5b1753d4681908e8efb0d1236b47c64e3835a670 100644 (file)
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2422,38 +2422,30 @@ void
 GetOperatorFromCompareType(Oid opclass, Oid rhstype, CompareType cmptype,
                           Oid *opid, StrategyNumber *strat)
 {
+   Oid         amid;
    Oid         opfamily;
    Oid         opcintype;
 
    Assert(cmptype == COMPARE_EQ || cmptype == COMPARE_OVERLAP || cmptype == COMPARE_CONTAINED_BY);
 
+   amid = get_opclass_method(opclass);
+
    *opid = InvalidOid;
 
    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 +2464,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 9827b27af8a6586c5a09a4eefafbaa4ea4bb5671..18f64db6e398077f16fe45301ddc79a6bf3268ec 100644 (file)
--- 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 2dac4bd363b383439607537c58868e45022d0e75..5f7613cc831580c4ceec69e1f603ebcd8073ee4c 100644 (file)
--- 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 67aab02ff76c5eba0bca16e1381b61ad25edc6fb..e9ad90d64a50392c6099696ec8075a3cc4e70826 100644 (file)
--- 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 eff019f7515165906d40e17fed53f0ad02ebdb75..db62a9ac0b4b877b1a51364995d036e0cc8aa3a0 100644 (file)
--- 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 c7db6defd3eacad4742f463e7d0eaeb6fd17951a..45b80e6b98ee2f90556e948abb89faea6866a79f 100644 (file)
--- 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,