Re: Rethinking opclass member checks and dependency strength - Mailing list pgsql-hackers

Tomas Vondra <tomas.vondra@2ndquadrant.com> writes:
> The latest version of this patch (from 2019/09/14) no longer applies,
> although maybe it's some issue with patch format (applying it using
> patch works fine, git am fails with "Patch format detection failed.").

Hm, seems to be just a trivial conflict against the copyright-date-update
patch.  Updated version attached.

> On Sat, Sep 14, 2019 at 07:01:33PM -0400, Tom Lane wrote:
>> This change results in a possibly surprising change in the expected output
>> for the 002_pg_dump.pl test: an optional support function that had been
>> created as part of CREATE OPERATOR CLASS will now be dumped as part of
>> ALTER OPERATOR FAMILY.  Maybe that's too surprising?  Another approach
>> that we could use is to give up the premise that soft dependencies are
>> always on the opfamily.  If we kept the dependencies pointing to the
>> same objects as before (opclass or opfamily) and only twiddled the
>> dependency strength, then pg_dump's output would not change.  Now,
>> this would possibly result in dropping a still-useful family member
>> if it were incorrectly tied to an opclass that's dropped --- but that
>> would have happened before, too.  I'm not quite sure if we really want
>> to editorialize on the user's decisions about which grouping to tie
>> family members to.

> I agree it's a bit weird to add a dependency on an opfamily and not the
> opclass. Not just because of the pg_dump weirdness, but doesn't it mean
> that after a DROP OPERATOR CLASS we might still reject a DROP OPERATOR
> because of the remaining opfamily dependency? (Haven't tried, so maybe
> that works fine).

I poked at the idea of retaining the user's decisions as to whether
a member object is a member of an individual opclass or an opfamily,
but soon realized that there's a big problem with that: we don't have
any ALTER OPERATOR CLASS ADD/DROP syntax, only ALTER OPERATOR FAMILY.
So there's no way to express the concept of "add this at the opclass
level", if you forgot to add it during initial opclass creation.

I suppose that some case could be made for adding such syntax, but
it seems like a significant amount of work, and in the end it seems
like it's better to trust the system to get these assignments right.
Letting the user do it doesn't add much except the opportunity
to shoot oneself in the foot.

> One minor comment from me is that maybe "amcheckmembers" is a bit
> misleading. In my mind "check" implies a plain passive check, not
> something that may actually tweak the dependency type.

Hmm.  I'm not wedded to that name, but do you have a better proposal?
The end goal (not realized in this patch, of course) is that these
callbacks would perform fairly thorough checking of opclass members,
missing only the ability to check that all required members are present.
So I don't want to name them something like "amfixdependencies", even
if that's all they're doing right now.

I see your point that "check" suggests a read-only operation, but
I'm not sure about a better verb.  I thought of "amvalidatemembers",
but that's not really much better than "check" is it?

            regards, tom lane

diff --git a/contrib/bloom/blutils.c b/contrib/bloom/blutils.c
index 23d959b..b016be1 100644
--- a/contrib/bloom/blutils.c
+++ b/contrib/bloom/blutils.c
@@ -134,6 +134,7 @@ blhandler(PG_FUNCTION_ARGS)
     amroutine->amproperty = NULL;
     amroutine->ambuildphasename = NULL;
     amroutine->amvalidate = blvalidate;
+    amroutine->amcheckmembers = NULL;
     amroutine->ambeginscan = blbeginscan;
     amroutine->amrescan = blrescan;
     amroutine->amgettuple = NULL;
diff --git a/doc/src/sgml/indexam.sgml b/doc/src/sgml/indexam.sgml
index dd54c68..2d16c6f 100644
--- a/doc/src/sgml/indexam.sgml
+++ b/doc/src/sgml/indexam.sgml
@@ -137,6 +137,7 @@ typedef struct IndexAmRoutine
     amproperty_function amproperty;     /* can be NULL */
     ambuildphasename_function ambuildphasename;   /* can be NULL */
     amvalidate_function amvalidate;
+    amcheckmembers_function amcheckmembers; /* can be NULL */
     ambeginscan_function ambeginscan;
     amrescan_function amrescan;
     amgettuple_function amgettuple;     /* can be NULL */
@@ -496,7 +497,48 @@ amvalidate (Oid opclassoid);
    the access method can reasonably do that.  For example, this might include
    testing that all required support functions are provided.
    The <function>amvalidate</function> function must return false if the opclass is
-   invalid.  Problems should be reported with <function>ereport</function> messages.
+   invalid.  Problems should be reported with <function>ereport</function>
+   messages, typically at <literal>INFO</literal> level.
+  </para>
+
+  <para>
+<programlisting>
+void
+amcheckmembers (Oid opfamilyoid,
+                Oid opclassoid,
+                List *operators,
+                List *functions);
+</programlisting>
+   Validate proposed operator and function members of an operator family,
+   so far as the access method can reasonably do that, and set their
+   dependency types if the default is not satisfactory.  This is called
+   during <command>CREATE OPERATOR CLASS</command> and during
+   <command>ALTER OPERATOR FAMILY ADD</command>; in the latter
+   case <parameter>opclassoid</parameter> is <literal>InvalidOid</literal>.
+   The <type>List</type> arguments are lists
+   of <structname>OpFamilyMember</structname> structs, as defined
+   in <filename>amapi.h</filename>.
+
+   Tests done by this function will typically be a subset of those
+   performed by <function>amvalidate</function>,
+   since <function>amcheckmembers</function> cannot assume that it is
+   seeing a complete set of members.  For example, it would be reasonable
+   to check the signature of a support function, but not to check whether
+   all required support functions are provided.  Any problems can be
+   reported by throwing an error.
+
+   The dependency-related fields of
+   the <structname>OpFamilyMember</structname> structs are initialized by
+   the core code to create hard dependencies on the opclass if this
+   is <command>CREATE OPERATOR CLASS</command>, or soft dependencies on the
+   opfamily if this is <command>ALTER OPERATOR FAMILY ADD</command>.
+   <function>amcheckmembers</function> can adjust these fields if some other
+   behavior is more appropriate.  For example, GIN, GiST, and SP-GiST
+   always set operator members to have soft dependencies on the opfamily,
+   since the connection between an operator and an opclass is relatively
+   weak in these index types; so it is reasonable to allow operator members
+   to be added and removed freely.  Optional support functions are typically
+   also given soft dependencies, so that they can be removed if necessary.
   </para>


diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index d89af78..ceea92f 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -114,6 +114,7 @@ brinhandler(PG_FUNCTION_ARGS)
     amroutine->amproperty = NULL;
     amroutine->ambuildphasename = NULL;
     amroutine->amvalidate = brinvalidate;
+    amroutine->amcheckmembers = NULL;
     amroutine->ambeginscan = brinbeginscan;
     amroutine->amrescan = brinrescan;
     amroutine->amgettuple = NULL;
diff --git a/src/backend/access/gin/ginutil.c b/src/backend/access/gin/ginutil.c
index 910f0bc..b06772d 100644
--- a/src/backend/access/gin/ginutil.c
+++ b/src/backend/access/gin/ginutil.c
@@ -66,6 +66,7 @@ ginhandler(PG_FUNCTION_ARGS)
     amroutine->amproperty = NULL;
     amroutine->ambuildphasename = NULL;
     amroutine->amvalidate = ginvalidate;
+    amroutine->amcheckmembers = gincheckmembers;
     amroutine->ambeginscan = ginbeginscan;
     amroutine->amrescan = ginrescan;
     amroutine->amgettuple = NULL;
diff --git a/src/backend/access/gin/ginvalidate.c b/src/backend/access/gin/ginvalidate.c
index 0b62e0a..f44aa4d 100644
--- a/src/backend/access/gin/ginvalidate.c
+++ b/src/backend/access/gin/ginvalidate.c
@@ -267,3 +267,64 @@ ginvalidate(Oid opclassoid)

     return result;
 }
+
+/*
+ * Prechecking function for adding operators/functions to a GIN opfamily.
+ */
+void
+gincheckmembers(Oid opfamilyoid,
+                Oid opclassoid,
+                List *operators,
+                List *functions)
+{
+    ListCell   *lc;
+
+    /*
+     * Operator members of a GIN opfamily should never have hard dependencies,
+     * since their connection to the opfamily depends only on what the support
+     * functions think, and that can be altered.  For consistency, we make all
+     * soft dependencies point to the opfamily, though a soft dependency on
+     * the opclass would work as well in the CREATE OPERATOR CLASS case.
+     */
+    foreach(lc, operators)
+    {
+        OpFamilyMember *op = (OpFamilyMember *) lfirst(lc);
+
+        op->ref_is_hard = false;
+        op->ref_is_family = true;
+        op->refobjid = opfamilyoid;
+    }
+
+    /*
+     * Required support functions should have hard dependencies.  Preferably
+     * those are just dependencies on the opclass, but if we're in ALTER
+     * OPERATOR FAMILY, we leave the dependency pointing at the whole
+     * opfamily.  (Given that GIN opclasses generally don't share opfamilies,
+     * it seems unlikely to be worth working harder.)
+     */
+    foreach(lc, functions)
+    {
+        OpFamilyMember *op = (OpFamilyMember *) lfirst(lc);
+
+        switch (op->number)
+        {
+            case GIN_EXTRACTVALUE_PROC:
+            case GIN_EXTRACTQUERY_PROC:
+                /* Required support function */
+                op->ref_is_hard = true;
+                break;
+            case GIN_COMPARE_PROC:
+            case GIN_CONSISTENT_PROC:
+            case GIN_COMPARE_PARTIAL_PROC:
+            case GIN_TRICONSISTENT_PROC:
+                /* Optional, so force it to be a soft family dependency */
+                op->ref_is_hard = false;
+                op->ref_is_family = true;
+                op->refobjid = opfamilyoid;
+                break;
+            default:
+                /* Probably we should throw error here */
+                break;
+        }
+    }
+}
diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c
index 5c9ad34..23721c3 100644
--- a/src/backend/access/gist/gist.c
+++ b/src/backend/access/gist/gist.c
@@ -87,6 +87,7 @@ gisthandler(PG_FUNCTION_ARGS)
     amroutine->amproperty = gistproperty;
     amroutine->ambuildphasename = NULL;
     amroutine->amvalidate = gistvalidate;
+    amroutine->amcheckmembers = gistcheckmembers;
     amroutine->ambeginscan = gistbeginscan;
     amroutine->amrescan = gistrescan;
     amroutine->amgettuple = gistgettuple;
diff --git a/src/backend/access/gist/gistvalidate.c b/src/backend/access/gist/gistvalidate.c
index 0c4fb8c..46c31c4 100644
--- a/src/backend/access/gist/gistvalidate.c
+++ b/src/backend/access/gist/gistvalidate.c
@@ -275,3 +275,68 @@ gistvalidate(Oid opclassoid)

     return result;
 }
+
+/*
+ * Prechecking function for adding operators/functions to a GiST opfamily.
+ */
+void
+gistcheckmembers(Oid opfamilyoid,
+                 Oid opclassoid,
+                 List *operators,
+                 List *functions)
+{
+    ListCell   *lc;
+
+    /*
+     * Operator members of a GiST opfamily should never have hard
+     * dependencies, since their connection to the opfamily depends only on
+     * what the support functions think, and that can be altered.  For
+     * consistency, we make all soft dependencies point to the opfamily,
+     * though a soft dependency on the opclass would work as well in the
+     * CREATE OPERATOR CLASS case.
+     */
+    foreach(lc, operators)
+    {
+        OpFamilyMember *op = (OpFamilyMember *) lfirst(lc);
+
+        op->ref_is_hard = false;
+        op->ref_is_family = true;
+        op->refobjid = opfamilyoid;
+    }
+
+    /*
+     * Required support functions should have hard dependencies.  Preferably
+     * those are just dependencies on the opclass, but if we're in ALTER
+     * OPERATOR FAMILY, we leave the dependency pointing at the whole
+     * opfamily.  (Given that GiST opclasses generally don't share opfamilies,
+     * it seems unlikely to be worth working harder.)
+     */
+    foreach(lc, functions)
+    {
+        OpFamilyMember *op = (OpFamilyMember *) lfirst(lc);
+
+        switch (op->number)
+        {
+            case GIST_CONSISTENT_PROC:
+            case GIST_UNION_PROC:
+            case GIST_PENALTY_PROC:
+            case GIST_PICKSPLIT_PROC:
+            case GIST_EQUAL_PROC:
+                /* Required support function */
+                op->ref_is_hard = true;
+                break;
+            case GIST_COMPRESS_PROC:
+            case GIST_DECOMPRESS_PROC:
+            case GIST_DISTANCE_PROC:
+            case GIST_FETCH_PROC:
+                /* Optional, so force it to be a soft family dependency */
+                op->ref_is_hard = false;
+                op->ref_is_family = true;
+                op->refobjid = opfamilyoid;
+                break;
+            default:
+                /* Probably we should throw error here */
+                break;
+        }
+    }
+}
diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c
index 4bb6efc..82d8336 100644
--- a/src/backend/access/hash/hash.c
+++ b/src/backend/access/hash/hash.c
@@ -85,6 +85,7 @@ hashhandler(PG_FUNCTION_ARGS)
     amroutine->amproperty = NULL;
     amroutine->ambuildphasename = NULL;
     amroutine->amvalidate = hashvalidate;
+    amroutine->amcheckmembers = hashcheckmembers;
     amroutine->ambeginscan = hashbeginscan;
     amroutine->amrescan = hashrescan;
     amroutine->amgettuple = hashgettuple;
diff --git a/src/backend/access/hash/hashvalidate.c b/src/backend/access/hash/hashvalidate.c
index 6346e65..5c8626a 100644
--- a/src/backend/access/hash/hashvalidate.c
+++ b/src/backend/access/hash/hashvalidate.c
@@ -16,6 +16,8 @@
 #include "access/amvalidate.h"
 #include "access/hash.h"
 #include "access/htup_details.h"
+#include "access/xact.h"
+#include "catalog/pg_am.h"
 #include "catalog/pg_amop.h"
 #include "catalog/pg_amproc.h"
 #include "catalog/pg_opclass.h"
@@ -25,6 +27,7 @@
 #include "parser/parse_coerce.h"
 #include "utils/builtins.h"
 #include "utils/fmgroids.h"
+#include "utils/lsyscache.h"
 #include "utils/regproc.h"
 #include "utils/syscache.h"

@@ -334,3 +337,96 @@ check_hash_func_signature(Oid funcid, int16 amprocnum, Oid argtype)
     ReleaseSysCache(tp);
     return result;
 }
+
+/*
+ * Prechecking function for adding operators/functions to a hash opfamily.
+ */
+void
+hashcheckmembers(Oid opfamilyoid,
+                 Oid opclassoid,
+                 List *operators,
+                 List *functions)
+{
+    Oid            opcintype;
+    ListCell   *lc;
+
+    /*
+     * Hash operators and required support functions are always "loose"
+     * members of the opfamily if they are cross-type.  If they are not
+     * cross-type, we prefer to tie them to the appropriate opclass ... but if
+     * the user hasn't created one, we can't do that, and must fall back to
+     * using the opfamily dependency.  (We mustn't force creation of an
+     * opclass in such a case, as leaving an incomplete opclass laying about
+     * would be bad.  Throwing an error is another undesirable alternative.)
+     *
+     * This behavior results in a bit of a dump/reload hazard, in that the
+     * order of restoring objects could affect what dependencies we end up
+     * with.  pg_dump's existing behavior will preserve the dependency choices
+     * in most cases, but not if a cross-type operator has been bound tightly
+     * into an opclass.  That's a mistake anyway, so silently "fixing" it
+     * isn't awful.
+     *
+     * Optional support functions are always "loose" family members.
+     *
+     * To avoid repeated lookups, we remember the most recently used opclass's
+     * input type.
+     */
+    if (OidIsValid(opclassoid))
+    {
+        /* During CREATE OPERATOR CLASS, need CCI to see the pg_opclass row */
+        CommandCounterIncrement();
+        opcintype = get_opclass_input_type(opclassoid);
+    }
+    else
+        opcintype = InvalidOid;
+
+    /*
+     * We handle operators and support functions almost identically, so rather
+     * than duplicate this code block, just join the lists.
+     */
+    foreach(lc, list_concat_copy(operators, functions))
+    {
+        OpFamilyMember *op = (OpFamilyMember *) lfirst(lc);
+
+        if (op->is_func && op->number != HASHSTANDARD_PROC)
+        {
+            /* Optional support proc, so always a soft family dependency */
+            op->ref_is_hard = false;
+            op->ref_is_family = true;
+            op->refobjid = opfamilyoid;
+        }
+        else if (op->lefttype != op->righttype)
+        {
+            /* Cross-type, so always a soft family dependency */
+            op->ref_is_hard = false;
+            op->ref_is_family = true;
+            op->refobjid = opfamilyoid;
+        }
+        else
+        {
+            /* Not cross-type; is there a suitable opclass? */
+            if (op->lefttype != opcintype)
+            {
+                /* Avoid repeating this expensive lookup, even if it fails */
+                opcintype = op->lefttype;
+                opclassoid = opclass_for_family_datatype(HASH_AM_OID,
+                                                         opfamilyoid,
+                                                         opcintype);
+            }
+            if (OidIsValid(opclassoid))
+            {
+                /* Hard dependency on opclass */
+                op->ref_is_hard = true;
+                op->ref_is_family = false;
+                op->refobjid = opclassoid;
+            }
+            else
+            {
+                /* We're stuck, so make a soft dependency on the opfamily */
+                op->ref_is_hard = false;
+                op->ref_is_family = true;
+                op->refobjid = opfamilyoid;
+            }
+        }
+    }
+}
diff --git a/src/backend/access/index/amvalidate.c b/src/backend/access/index/amvalidate.c
index 3eae6aa..fa1f9d8 100644
--- a/src/backend/access/index/amvalidate.c
+++ b/src/backend/access/index/amvalidate.c
@@ -1,7 +1,8 @@
 /*-------------------------------------------------------------------------
  *
  * amvalidate.c
- *      Support routines for index access methods' amvalidate functions.
+ *      Support routines for index access methods' amvalidate and
+ *      amcheckmembers functions.
  *
  * Copyright (c) 2016-2020, PostgreSQL Global Development Group
  *
@@ -211,21 +212,28 @@ check_amop_signature(Oid opno, Oid restype, Oid lefttype, Oid righttype)
 }

 /*
- * Is the datatype a legitimate input type for the btree opfamily?
+ * Get the OID of the opclass belonging to an opfamily and accepting
+ * the specified type as input type.  Returns InvalidOid if no such opclass.
+ *
+ * If there is more than one such opclass, you get a random one of them.
+ * Since that shouldn't happen, we don't waste cycles checking.
+ *
+ * We could look up the AM's OID from the opfamily, but all existing callers
+ * know that or can get it without an extra lookup, so we make them pass it.
  */
-bool
-opfamily_can_sort_type(Oid opfamilyoid, Oid datatypeoid)
+Oid
+opclass_for_family_datatype(Oid amoid, Oid opfamilyoid, Oid datatypeoid)
 {
-    bool        result = false;
+    Oid            result = InvalidOid;
     CatCList   *opclist;
     int            i;

     /*
-     * We search through all btree opclasses to see if one matches.  This is a
-     * bit inefficient but there is no better index available.  It also saves
-     * making an explicit check that the opfamily belongs to btree.
+     * We search through all the AM's opclasses to see if one matches.  This
+     * is a bit inefficient but there is no better index available.  It also
+     * saves making an explicit check that the opfamily belongs to the AM.
      */
-    opclist = SearchSysCacheList1(CLAAMNAMENSP, ObjectIdGetDatum(BTREE_AM_OID));
+    opclist = SearchSysCacheList1(CLAAMNAMENSP, ObjectIdGetDatum(amoid));

     for (i = 0; i < opclist->n_members; i++)
     {
@@ -235,7 +243,7 @@ opfamily_can_sort_type(Oid opfamilyoid, Oid datatypeoid)
         if (classform->opcfamily == opfamilyoid &&
             classform->opcintype == datatypeoid)
         {
-            result = true;
+            result = classform->oid;
             break;
         }
     }
@@ -244,3 +252,14 @@ opfamily_can_sort_type(Oid opfamilyoid, Oid datatypeoid)

     return result;
 }
+
+/*
+ * Is the datatype a legitimate input type for the btree opfamily?
+ */
+bool
+opfamily_can_sort_type(Oid opfamilyoid, Oid datatypeoid)
+{
+    return OidIsValid(opclass_for_family_datatype(BTREE_AM_OID,
+                                                  opfamilyoid,
+                                                  datatypeoid));
+}
diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index 8376a5e..79bdbd5 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -134,6 +134,7 @@ bthandler(PG_FUNCTION_ARGS)
     amroutine->amproperty = btproperty;
     amroutine->ambuildphasename = btbuildphasename;
     amroutine->amvalidate = btvalidate;
+    amroutine->amcheckmembers = btcheckmembers;
     amroutine->ambeginscan = btbeginscan;
     amroutine->amrescan = btrescan;
     amroutine->amgettuple = btgettuple;
diff --git a/src/backend/access/nbtree/nbtvalidate.c b/src/backend/access/nbtree/nbtvalidate.c
index ff634b1..3a0c089 100644
--- a/src/backend/access/nbtree/nbtvalidate.c
+++ b/src/backend/access/nbtree/nbtvalidate.c
@@ -16,12 +16,15 @@
 #include "access/amvalidate.h"
 #include "access/htup_details.h"
 #include "access/nbtree.h"
+#include "access/xact.h"
+#include "catalog/pg_am.h"
 #include "catalog/pg_amop.h"
 #include "catalog/pg_amproc.h"
 #include "catalog/pg_opclass.h"
 #include "catalog/pg_opfamily.h"
 #include "catalog/pg_type.h"
 #include "utils/builtins.h"
+#include "utils/lsyscache.h"
 #include "utils/regproc.h"
 #include "utils/syscache.h"

@@ -275,3 +278,96 @@ btvalidate(Oid opclassoid)

     return result;
 }
+
+/*
+ * Prechecking function for adding operators/functions to a btree opfamily.
+ */
+void
+btcheckmembers(Oid opfamilyoid,
+               Oid opclassoid,
+               List *operators,
+               List *functions)
+{
+    Oid            opcintype;
+    ListCell   *lc;
+
+    /*
+     * Btree operators and comparison support functions are always "loose"
+     * members of the opfamily if they are cross-type.  If they are not
+     * cross-type, we prefer to tie them to the appropriate opclass ... but if
+     * the user hasn't created one, we can't do that, and must fall back to
+     * using the opfamily dependency.  (We mustn't force creation of an
+     * opclass in such a case, as leaving an incomplete opclass laying about
+     * would be bad.  Throwing an error is another undesirable alternative.)
+     *
+     * This behavior results in a bit of a dump/reload hazard, in that the
+     * order of restoring objects could affect what dependencies we end up
+     * with.  pg_dump's existing behavior will preserve the dependency choices
+     * in most cases, but not if a cross-type operator has been bound tightly
+     * into an opclass.  That's a mistake anyway, so silently "fixing" it
+     * isn't awful.
+     *
+     * Optional support functions are always "loose" family members.
+     *
+     * To avoid repeated lookups, we remember the most recently used opclass's
+     * input type.
+     */
+    if (OidIsValid(opclassoid))
+    {
+        /* During CREATE OPERATOR CLASS, need CCI to see the pg_opclass row */
+        CommandCounterIncrement();
+        opcintype = get_opclass_input_type(opclassoid);
+    }
+    else
+        opcintype = InvalidOid;
+
+    /*
+     * We handle operators and support functions almost identically, so rather
+     * than duplicate this code block, just join the lists.
+     */
+    foreach(lc, list_concat_copy(operators, functions))
+    {
+        OpFamilyMember *op = (OpFamilyMember *) lfirst(lc);
+
+        if (op->is_func && op->number != BTORDER_PROC)
+        {
+            /* Optional support proc, so always a soft family dependency */
+            op->ref_is_hard = false;
+            op->ref_is_family = true;
+            op->refobjid = opfamilyoid;
+        }
+        else if (op->lefttype != op->righttype)
+        {
+            /* Cross-type, so always a soft family dependency */
+            op->ref_is_hard = false;
+            op->ref_is_family = true;
+            op->refobjid = opfamilyoid;
+        }
+        else
+        {
+            /* Not cross-type; is there a suitable opclass? */
+            if (op->lefttype != opcintype)
+            {
+                /* Avoid repeating this expensive lookup, even if it fails */
+                opcintype = op->lefttype;
+                opclassoid = opclass_for_family_datatype(BTREE_AM_OID,
+                                                         opfamilyoid,
+                                                         opcintype);
+            }
+            if (OidIsValid(opclassoid))
+            {
+                /* Hard dependency on opclass */
+                op->ref_is_hard = true;
+                op->ref_is_family = false;
+                op->refobjid = opclassoid;
+            }
+            else
+            {
+                /* We're stuck, so make a soft dependency on the opfamily */
+                op->ref_is_hard = false;
+                op->ref_is_family = true;
+                op->refobjid = opfamilyoid;
+            }
+        }
+    }
+}
diff --git a/src/backend/access/spgist/spgutils.c b/src/backend/access/spgist/spgutils.c
index d715908..ad2fa8c 100644
--- a/src/backend/access/spgist/spgutils.c
+++ b/src/backend/access/spgist/spgutils.c
@@ -69,6 +69,7 @@ spghandler(PG_FUNCTION_ARGS)
     amroutine->amproperty = spgproperty;
     amroutine->ambuildphasename = NULL;
     amroutine->amvalidate = spgvalidate;
+    amroutine->amcheckmembers = spgcheckmembers;
     amroutine->ambeginscan = spgbeginscan;
     amroutine->amrescan = spgrescan;
     amroutine->amgettuple = spggettuple;
diff --git a/src/backend/access/spgist/spgvalidate.c b/src/backend/access/spgist/spgvalidate.c
index e316d6e..9a9e301 100644
--- a/src/backend/access/spgist/spgvalidate.c
+++ b/src/backend/access/spgist/spgvalidate.c
@@ -298,3 +298,65 @@ spgvalidate(Oid opclassoid)

     return result;
 }
+
+/*
+ * Prechecking function for adding operators/functions to an SP-GiST opfamily.
+ */
+void
+spgcheckmembers(Oid opfamilyoid,
+                Oid opclassoid,
+                List *operators,
+                List *functions)
+{
+    ListCell   *lc;
+
+    /*
+     * Operator members of an SP-GiST opfamily should never have hard
+     * dependencies, since their connection to the opfamily depends only on
+     * what the support functions think, and that can be altered.  For
+     * consistency, we make all soft dependencies point to the opfamily,
+     * though a soft dependency on the opclass would work as well in the
+     * CREATE OPERATOR CLASS case.
+     */
+    foreach(lc, operators)
+    {
+        OpFamilyMember *op = (OpFamilyMember *) lfirst(lc);
+
+        op->ref_is_hard = false;
+        op->ref_is_family = true;
+        op->refobjid = opfamilyoid;
+    }
+
+    /*
+     * Required support functions should have hard dependencies.  Preferably
+     * those are just dependencies on the opclass, but if we're in ALTER
+     * OPERATOR FAMILY, we leave the dependency pointing at the whole
+     * opfamily.  (Given that SP-GiST opclasses generally don't share
+     * opfamilies, it seems unlikely to be worth working harder.)
+     */
+    foreach(lc, functions)
+    {
+        OpFamilyMember *op = (OpFamilyMember *) lfirst(lc);
+
+        switch (op->number)
+        {
+            case SPGIST_CONFIG_PROC:
+            case SPGIST_CHOOSE_PROC:
+            case SPGIST_PICKSPLIT_PROC:
+            case SPGIST_INNER_CONSISTENT_PROC:
+            case SPGIST_LEAF_CONSISTENT_PROC:
+                /* Required support function */
+                op->ref_is_hard = true;
+                break;
+            case SPGIST_COMPRESS_PROC:
+                /* Optional, so force it to be a soft family dependency */
+                op->ref_is_hard = false;
+                op->ref_is_family = true;
+                op->refobjid = opfamilyoid;
+                break;
+            default:
+                /* Probably we should throw error here */
+                break;
+        }
+    }
+}
diff --git a/src/backend/commands/opclasscmds.c b/src/backend/commands/opclasscmds.c
index e2c6de4..9706208 100644
--- a/src/backend/commands/opclasscmds.c
+++ b/src/backend/commands/opclasscmds.c
@@ -27,7 +27,6 @@
 #include "catalog/dependency.h"
 #include "catalog/indexing.h"
 #include "catalog/objectaccess.h"
-#include "catalog/opfam_internal.h"
 #include "catalog/pg_am.h"
 #include "catalog/pg_amop.h"
 #include "catalog/pg_amproc.h"
@@ -61,12 +60,10 @@ static void AlterOpFamilyDrop(AlterOpFamilyStmt *stmt,
 static void processTypesSpec(List *args, Oid *lefttype, Oid *righttype);
 static void assignOperTypes(OpFamilyMember *member, Oid amoid, Oid typeoid);
 static void assignProcTypes(OpFamilyMember *member, Oid amoid, Oid typeoid);
-static void addFamilyMember(List **list, OpFamilyMember *member, bool isProc);
-static void storeOperators(List *opfamilyname, Oid amoid,
-                           Oid opfamilyoid, Oid opclassoid,
+static void addFamilyMember(List **list, OpFamilyMember *member);
+static void storeOperators(List *opfamilyname, Oid amoid, Oid opfamilyoid,
                            List *operators, bool isAdd);
-static void storeProcedures(List *opfamilyname, Oid amoid,
-                            Oid opfamilyoid, Oid opclassoid,
+static void storeProcedures(List *opfamilyname, Oid amoid, Oid opfamilyoid,
                             List *procedures, bool isAdd);
 static void dropOperators(List *opfamilyname, Oid amoid, Oid opfamilyoid,
                           List *operators);
@@ -515,11 +512,12 @@ DefineOpClass(CreateOpClassStmt *stmt)

                 /* Save the info */
                 member = (OpFamilyMember *) palloc0(sizeof(OpFamilyMember));
+                member->is_func = false;
                 member->object = operOid;
                 member->number = item->number;
                 member->sortfamily = sortfamilyOid;
                 assignOperTypes(member, amoid, typeoid);
-                addFamilyMember(&operators, member, false);
+                addFamilyMember(&operators, member);
                 break;
             case OPCLASS_ITEM_FUNCTION:
                 if (item->number <= 0 || item->number > maxProcNumber)
@@ -539,6 +537,7 @@ DefineOpClass(CreateOpClassStmt *stmt)

                 /* Save the info */
                 member = (OpFamilyMember *) palloc0(sizeof(OpFamilyMember));
+                member->is_func = true;
                 member->object = funcOid;
                 member->number = item->number;

@@ -548,7 +547,7 @@ DefineOpClass(CreateOpClassStmt *stmt)
                                      &member->lefttype, &member->righttype);

                 assignProcTypes(member, amoid, typeoid);
-                addFamilyMember(&procedures, member, true);
+                addFamilyMember(&procedures, member);
                 break;
             case OPCLASS_ITEM_STORAGETYPE:
                 if (OidIsValid(storageoid))
@@ -661,13 +660,45 @@ DefineOpClass(CreateOpClassStmt *stmt)
     heap_freetuple(tup);

     /*
+     * Now that we have the opclass OID, set up default dependency info for
+     * the pg_amop and pg_amproc entries.  Historically, CREATE OPERATOR CLASS
+     * has created hard dependencies on the opclass, so that's what we use.
+     */
+    foreach(l, operators)
+    {
+        OpFamilyMember *op = (OpFamilyMember *) lfirst(l);
+
+        op->ref_is_hard = true;
+        op->ref_is_family = false;
+        op->refobjid = opclassoid;
+    }
+    foreach(l, procedures)
+    {
+        OpFamilyMember *proc = (OpFamilyMember *) lfirst(l);
+
+        proc->ref_is_hard = true;
+        proc->ref_is_family = false;
+        proc->refobjid = opclassoid;
+    }
+
+    /*
+     * Let the index AM editorialize on the dependency choices.  It could also
+     * do further validation on the operators and functions, if it likes.
+     */
+    if (amroutine->amcheckmembers)
+        amroutine->amcheckmembers(opfamilyoid,
+                                  opclassoid,
+                                  operators,
+                                  procedures);
+
+    /*
      * Now add tuples to pg_amop and pg_amproc tying in the operators and
      * functions.  Dependencies on them are inserted, too.
      */
     storeOperators(stmt->opfamilyname, amoid, opfamilyoid,
-                   opclassoid, operators, false);
+                   operators, false);
     storeProcedures(stmt->opfamilyname, amoid, opfamilyoid,
-                    opclassoid, procedures, false);
+                    procedures, false);

     /* let event triggers know what happened */
     EventTriggerCollectCreateOpClass(stmt, opclassoid, operators, procedures);
@@ -836,6 +867,7 @@ static void
 AlterOpFamilyAdd(AlterOpFamilyStmt *stmt, Oid amoid, Oid opfamilyoid,
                  int maxOpNumber, int maxProcNumber, List *items)
 {
+    IndexAmRoutine *amroutine = GetIndexAmRoutineByAmId(amoid, false);
     List       *operators;        /* OpFamilyMember list for operators */
     List       *procedures;        /* OpFamilyMember list for support procs */
     ListCell   *l;
@@ -894,11 +926,17 @@ AlterOpFamilyAdd(AlterOpFamilyStmt *stmt, Oid amoid, Oid opfamilyoid,

                 /* Save the info */
                 member = (OpFamilyMember *) palloc0(sizeof(OpFamilyMember));
+                member->is_func = false;
                 member->object = operOid;
                 member->number = item->number;
                 member->sortfamily = sortfamilyOid;
+                /* We can set up dependency fields immediately */
+                /* Historically, ALTER ADD has created soft dependencies */
+                member->ref_is_hard = false;
+                member->ref_is_family = true;
+                member->refobjid = opfamilyoid;
                 assignOperTypes(member, amoid, InvalidOid);
-                addFamilyMember(&operators, member, false);
+                addFamilyMember(&operators, member);
                 break;
             case OPCLASS_ITEM_FUNCTION:
                 if (item->number <= 0 || item->number > maxProcNumber)
@@ -918,8 +956,14 @@ AlterOpFamilyAdd(AlterOpFamilyStmt *stmt, Oid amoid, Oid opfamilyoid,

                 /* Save the info */
                 member = (OpFamilyMember *) palloc0(sizeof(OpFamilyMember));
+                member->is_func = true;
                 member->object = funcOid;
                 member->number = item->number;
+                /* We can set up dependency fields immediately */
+                /* Historically, ALTER ADD has created soft dependencies */
+                member->ref_is_hard = false;
+                member->ref_is_family = true;
+                member->refobjid = opfamilyoid;

                 /* allow overriding of the function's actual arg types */
                 if (item->class_args)
@@ -927,7 +971,7 @@ AlterOpFamilyAdd(AlterOpFamilyStmt *stmt, Oid amoid, Oid opfamilyoid,
                                      &member->lefttype, &member->righttype);

                 assignProcTypes(member, amoid, InvalidOid);
-                addFamilyMember(&procedures, member, true);
+                addFamilyMember(&procedures, member);
                 break;
             case OPCLASS_ITEM_STORAGETYPE:
                 ereport(ERROR,
@@ -941,13 +985,23 @@ AlterOpFamilyAdd(AlterOpFamilyStmt *stmt, Oid amoid, Oid opfamilyoid,
     }

     /*
+     * Let the index AM editorialize on the dependency choices.  It could also
+     * do further validation on the operators and functions, if it likes.
+     */
+    if (amroutine->amcheckmembers)
+        amroutine->amcheckmembers(opfamilyoid,
+                                  InvalidOid,    /* no specific opclass */
+                                  operators,
+                                  procedures);
+
+    /*
      * Add tuples to pg_amop and pg_amproc tying in the operators and
      * functions.  Dependencies on them are inserted, too.
      */
     storeOperators(stmt->opfamilyname, amoid, opfamilyoid,
-                   InvalidOid, operators, true);
+                   operators, true);
     storeProcedures(stmt->opfamilyname, amoid, opfamilyoid,
-                    InvalidOid, procedures, true);
+                    procedures, true);

     /* make information available to event triggers */
     EventTriggerCollectAlterOpFam(stmt, opfamilyoid,
@@ -990,10 +1044,11 @@ AlterOpFamilyDrop(AlterOpFamilyStmt *stmt, Oid amoid, Oid opfamilyoid,
                 processTypesSpec(item->class_args, &lefttype, &righttype);
                 /* Save the info */
                 member = (OpFamilyMember *) palloc0(sizeof(OpFamilyMember));
+                member->is_func = false;
                 member->number = item->number;
                 member->lefttype = lefttype;
                 member->righttype = righttype;
-                addFamilyMember(&operators, member, false);
+                addFamilyMember(&operators, member);
                 break;
             case OPCLASS_ITEM_FUNCTION:
                 if (item->number <= 0 || item->number > maxProcNumber)
@@ -1005,10 +1060,11 @@ AlterOpFamilyDrop(AlterOpFamilyStmt *stmt, Oid amoid, Oid opfamilyoid,
                 processTypesSpec(item->class_args, &lefttype, &righttype);
                 /* Save the info */
                 member = (OpFamilyMember *) palloc0(sizeof(OpFamilyMember));
+                member->is_func = true;
                 member->number = item->number;
                 member->lefttype = lefttype;
                 member->righttype = righttype;
-                addFamilyMember(&procedures, member, true);
+                addFamilyMember(&procedures, member);
                 break;
             case OPCLASS_ITEM_STORAGETYPE:
                 /* grammar prevents this from appearing */
@@ -1263,7 +1319,7 @@ assignProcTypes(OpFamilyMember *member, Oid amoid, Oid typeoid)
  * duplicated strategy or proc number.
  */
 static void
-addFamilyMember(List **list, OpFamilyMember *member, bool isProc)
+addFamilyMember(List **list, OpFamilyMember *member)
 {
     ListCell   *l;

@@ -1275,7 +1331,7 @@ addFamilyMember(List **list, OpFamilyMember *member, bool isProc)
             old->lefttype == member->lefttype &&
             old->righttype == member->righttype)
         {
-            if (isProc)
+            if (member->is_func)
                 ereport(ERROR,
                         (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
                          errmsg("function number %d for (%s,%s) appears more than once",
@@ -1297,13 +1353,10 @@ addFamilyMember(List **list, OpFamilyMember *member, bool isProc)
 /*
  * Dump the operators to pg_amop
  *
- * We also make dependency entries in pg_depend for the opfamily entries.
- * If opclassoid is valid then make an INTERNAL dependency on that opclass,
- * else make an AUTO dependency on the opfamily.
+ * We also make dependency entries in pg_depend for the pg_amop entries.
  */
 static void
-storeOperators(List *opfamilyname, Oid amoid,
-               Oid opfamilyoid, Oid opclassoid,
+storeOperators(List *opfamilyname, Oid amoid, Oid opfamilyoid,
                List *operators, bool isAdd)
 {
     Relation    rel;
@@ -1373,28 +1426,17 @@ storeOperators(List *opfamilyname, Oid amoid,
         referenced.objectId = op->object;
         referenced.objectSubId = 0;

-        if (OidIsValid(opclassoid))
-        {
-            /* if contained in an opclass, use a NORMAL dep on operator */
-            recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
+        /* see comments in amapi.h about dependency strength */
+        recordDependencyOn(&myself, &referenced,
+                           op->ref_is_hard ? DEPENDENCY_NORMAL : DEPENDENCY_AUTO);

-            /* ... and an INTERNAL dep on the opclass */
-            referenced.classId = OperatorClassRelationId;
-            referenced.objectId = opclassoid;
-            referenced.objectSubId = 0;
-            recordDependencyOn(&myself, &referenced, DEPENDENCY_INTERNAL);
-        }
-        else
-        {
-            /* if "loose" in the opfamily, use a AUTO dep on operator */
-            recordDependencyOn(&myself, &referenced, DEPENDENCY_AUTO);
+        referenced.classId = op->ref_is_family ? OperatorFamilyRelationId :
+            OperatorClassRelationId;
+        referenced.objectId = op->refobjid;
+        referenced.objectSubId = 0;

-            /* ... and an AUTO dep on the opfamily */
-            referenced.classId = OperatorFamilyRelationId;
-            referenced.objectId = opfamilyoid;
-            referenced.objectSubId = 0;
-            recordDependencyOn(&myself, &referenced, DEPENDENCY_AUTO);
-        }
+        recordDependencyOn(&myself, &referenced,
+                           op->ref_is_hard ? DEPENDENCY_INTERNAL : DEPENDENCY_AUTO);

         /* A search operator also needs a dep on the referenced opfamily */
         if (OidIsValid(op->sortfamily))
@@ -1402,8 +1444,11 @@ storeOperators(List *opfamilyname, Oid amoid,
             referenced.classId = OperatorFamilyRelationId;
             referenced.objectId = op->sortfamily;
             referenced.objectSubId = 0;
-            recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
+
+            recordDependencyOn(&myself, &referenced,
+                               op->ref_is_hard ? DEPENDENCY_NORMAL : DEPENDENCY_AUTO);
         }
+
         /* Post create hook of this access method operator */
         InvokeObjectPostCreateHook(AccessMethodOperatorRelationId,
                                    entryoid, 0);
@@ -1415,13 +1460,10 @@ storeOperators(List *opfamilyname, Oid amoid,
 /*
  * Dump the procedures (support routines) to pg_amproc
  *
- * We also make dependency entries in pg_depend for the opfamily entries.
- * If opclassoid is valid then make an INTERNAL dependency on that opclass,
- * else make an AUTO dependency on the opfamily.
+ * We also make dependency entries in pg_depend for the pg_amproc entries.
  */
 static void
-storeProcedures(List *opfamilyname, Oid amoid,
-                Oid opfamilyoid, Oid opclassoid,
+storeProcedures(List *opfamilyname, Oid amoid, Oid opfamilyoid,
                 List *procedures, bool isAdd)
 {
     Relation    rel;
@@ -1485,28 +1527,18 @@ storeProcedures(List *opfamilyname, Oid amoid,
         referenced.objectId = proc->object;
         referenced.objectSubId = 0;

-        if (OidIsValid(opclassoid))
-        {
-            /* if contained in an opclass, use a NORMAL dep on procedure */
-            recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
+        /* see comments in amapi.h about dependency strength */
+        recordDependencyOn(&myself, &referenced,
+                           proc->ref_is_hard ? DEPENDENCY_NORMAL : DEPENDENCY_AUTO);

-            /* ... and an INTERNAL dep on the opclass */
-            referenced.classId = OperatorClassRelationId;
-            referenced.objectId = opclassoid;
-            referenced.objectSubId = 0;
-            recordDependencyOn(&myself, &referenced, DEPENDENCY_INTERNAL);
-        }
-        else
-        {
-            /* if "loose" in the opfamily, use a AUTO dep on procedure */
-            recordDependencyOn(&myself, &referenced, DEPENDENCY_AUTO);
+        referenced.classId = proc->ref_is_family ? OperatorFamilyRelationId :
+            OperatorClassRelationId;
+        referenced.objectId = proc->refobjid;
+        referenced.objectSubId = 0;
+
+        recordDependencyOn(&myself, &referenced,
+                           proc->ref_is_hard ? DEPENDENCY_INTERNAL : DEPENDENCY_AUTO);

-            /* ... and an AUTO dep on the opfamily */
-            referenced.classId = OperatorFamilyRelationId;
-            referenced.objectId = opfamilyoid;
-            referenced.objectSubId = 0;
-            recordDependencyOn(&myself, &referenced, DEPENDENCY_AUTO);
-        }
         /* Post create hook of access method procedure */
         InvokeObjectPostCreateHook(AccessMethodProcedureRelationId,
                                    entryoid, 0);
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 4a9764c..3082a7b 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -523,6 +523,7 @@ my %tests = (
                          OPERATOR 5 >(bigint,int4),
                          FUNCTION 1 (int4, int4) btint4cmp(int4,int4),
                          FUNCTION 2 (int4, int4) btint4sortsupport(internal);',
+        # note: it's correct that btint8sortsupport is included here:
         regexp => qr/^
             \QALTER OPERATOR FAMILY dump_test.op_family USING btree ADD\E\n\s+
             \QOPERATOR 1 <(bigint,integer) ,\E\n\s+
@@ -531,6 +532,7 @@ my %tests = (
             \QOPERATOR 4 >=(bigint,integer) ,\E\n\s+
             \QOPERATOR 5 >(bigint,integer) ,\E\n\s+
             \QFUNCTION 1 (integer, integer) btint4cmp(integer,integer) ,\E\n\s+
+            \QFUNCTION 2 (bigint, bigint) btint8sortsupport(internal) ,\E\n\s+
             \QFUNCTION 2 (integer, integer) btint4sortsupport(internal);\E
             /xm,
         like =>
@@ -1555,6 +1557,7 @@ my %tests = (
                          OPERATOR 5 >(bigint,bigint),
                          FUNCTION 1 btint8cmp(bigint,bigint),
                          FUNCTION 2 btint8sortsupport(internal);',
+        # note: it's correct that btint8sortsupport isn't included here:
         regexp => qr/^
             \QCREATE OPERATOR CLASS dump_test.op_class\E\n\s+
             \QFOR TYPE bigint USING btree FAMILY dump_test.op_family AS\E\n\s+
@@ -1563,8 +1566,7 @@ my %tests = (
             \QOPERATOR 3 =(bigint,bigint) ,\E\n\s+
             \QOPERATOR 4 >=(bigint,bigint) ,\E\n\s+
             \QOPERATOR 5 >(bigint,bigint) ,\E\n\s+
-            \QFUNCTION 1 (bigint, bigint) btint8cmp(bigint,bigint) ,\E\n\s+
-            \QFUNCTION 2 (bigint, bigint) btint8sortsupport(internal);\E
+            \QFUNCTION 1 (bigint, bigint) btint8cmp(bigint,bigint);\E
             /xm,
         like =>
           { %full_runs, %dump_test_schema_runs, section_pre_data => 1, },
diff --git a/src/include/access/amapi.h b/src/include/access/amapi.h
index d2a49e8..b142aee 100644
--- a/src/include/access/amapi.h
+++ b/src/include/access/amapi.h
@@ -54,6 +54,42 @@ typedef enum IndexAMProperty
     AMPROP_CAN_INCLUDE
 } IndexAMProperty;

+/*
+ * We use lists of this struct type to keep track of both operators and
+ * support functions while building or adding to an opclass or opfamily.
+ * amcheckmembers functions receive lists of these structs, and are allowed
+ * to alter their "ref" fields.
+ *
+ * The "ref" fields define how the pg_amop or pg_amproc entry should depend
+ * on the associated objects (that is, which dependency type to use, and
+ * which opclass or opfamily it should depend on).
+ *
+ * If ref_is_hard is true, the entry will have a NORMAL dependency on the
+ * operator or support func, and an INTERNAL dependency on the opclass or
+ * opfamily.  This forces the opclass or opfamily to be dropped if the
+ * operator or support func is dropped, and requires the CASCADE option
+ * to do so.  Nor will ALTER OPERATOR FAMILY DROP be allowed.  This is
+ * the right behavior for objects that are essential to an opclass.
+ *
+ * If ref_is_hard is false, the entry will have an AUTO dependency on the
+ * operator or support func, and also an AUTO dependency on the opclass or
+ * opfamily.  This allows ALTER OPERATOR FAMILY DROP, and causes that to
+ * happen automatically if the operator or support func is dropped.  This
+ * is the right behavior for inessential ("loose") objects.
+ */
+typedef struct OpFamilyMember
+{
+    bool        is_func;        /* is this an operator, or support func? */
+    Oid            object;            /* operator or support func's OID */
+    int            number;            /* strategy or support func number */
+    Oid            lefttype;        /* lefttype */
+    Oid            righttype;        /* righttype */
+    Oid            sortfamily;        /* ordering operator's sort opfamily, or 0 */
+    bool        ref_is_hard;    /* hard or soft dependency? */
+    bool        ref_is_family;    /* is dependency on opclass or opfamily? */
+    Oid            refobjid;        /* OID of opclass or opfamily */
+} OpFamilyMember;
+

 /*
  * Callback function signatures --- see indexam.sgml for more info.
@@ -114,6 +150,12 @@ typedef char *(*ambuildphasename_function) (int64 phasenum);
 /* validate definition of an opclass for this AM */
 typedef bool (*amvalidate_function) (Oid opclassoid);

+/* validate operators and support functions to be added to an opclass/family */
+typedef void (*amcheckmembers_function) (Oid opfamilyoid,
+                                         Oid opclassoid,
+                                         List *operators,
+                                         List *functions);
+
 /* prepare for index scan */
 typedef IndexScanDesc (*ambeginscan_function) (Relation indexRelation,
                                                int nkeys,
@@ -218,6 +260,7 @@ typedef struct IndexAmRoutine
     amproperty_function amproperty; /* can be NULL */
     ambuildphasename_function ambuildphasename; /* can be NULL */
     amvalidate_function amvalidate;
+    amcheckmembers_function amcheckmembers; /* can be NULL */
     ambeginscan_function ambeginscan;
     amrescan_function amrescan;
     amgettuple_function amgettuple; /* can be NULL */
diff --git a/src/include/access/amvalidate.h b/src/include/access/amvalidate.h
index c6c60e0..707a705 100644
--- a/src/include/access/amvalidate.h
+++ b/src/include/access/amvalidate.h
@@ -1,7 +1,8 @@
 /*-------------------------------------------------------------------------
  *
  * amvalidate.h
- *      Support routines for index access methods' amvalidate functions.
+ *      Support routines for index access methods' amvalidate and
+ *      amcheckmembers functions.
  *
  * Copyright (c) 2016-2020, PostgreSQL Global Development Group
  *
@@ -31,6 +32,8 @@ extern bool check_amproc_signature(Oid funcid, Oid restype, bool exact,
                                    int minargs, int maxargs,...);
 extern bool check_amop_signature(Oid opno, Oid restype,
                                  Oid lefttype, Oid righttype);
+extern Oid    opclass_for_family_datatype(Oid amoid, Oid opfamilyoid,
+                                        Oid datatypeoid);
 extern bool opfamily_can_sort_type(Oid opfamilyoid, Oid datatypeoid);

 #endif                            /* AMVALIDATE_H */
diff --git a/src/include/access/gin_private.h b/src/include/access/gin_private.h
index a136f7f..352b8db 100644
--- a/src/include/access/gin_private.h
+++ b/src/include/access/gin_private.h
@@ -393,6 +393,10 @@ extern ItemPointer ginVacuumItemPointers(GinVacuumState *gvs,

 /* ginvalidate.c */
 extern bool ginvalidate(Oid opclassoid);
+extern void gincheckmembers(Oid opfamilyoid,
+                            Oid opclassoid,
+                            List *operators,
+                            List *functions);

 /* ginbulk.c */
 typedef struct GinEntryAccumulator
diff --git a/src/include/access/gist_private.h b/src/include/access/gist_private.h
index 18f2b0d..86cf332 100644
--- a/src/include/access/gist_private.h
+++ b/src/include/access/gist_private.h
@@ -462,6 +462,10 @@ extern bool gistcanreturn(Relation index, int attno);

 /* gistvalidate.c */
 extern bool gistvalidate(Oid opclassoid);
+extern void gistcheckmembers(Oid opfamilyoid,
+                             Oid opclassoid,
+                             List *operators,
+                             List *functions);

 /* gistutil.c */

diff --git a/src/include/access/hash.h b/src/include/access/hash.h
index 9fc0696..173cbf6 100644
--- a/src/include/access/hash.h
+++ b/src/include/access/hash.h
@@ -378,6 +378,10 @@ extern IndexBulkDeleteResult *hashvacuumcleanup(IndexVacuumInfo *info,
                                                 IndexBulkDeleteResult *stats);
 extern bytea *hashoptions(Datum reloptions, bool validate);
 extern bool hashvalidate(Oid opclassoid);
+extern void hashcheckmembers(Oid opfamilyoid,
+                             Oid opclassoid,
+                             List *operators,
+                             List *functions);

 /* private routines */

diff --git a/src/include/access/nbtree.h b/src/include/access/nbtree.h
index f90ee3a..11afb04 100644
--- a/src/include/access/nbtree.h
+++ b/src/include/access/nbtree.h
@@ -835,6 +835,10 @@ extern void _bt_check_third_page(Relation rel, Relation heap,
  * prototypes for functions in nbtvalidate.c
  */
 extern bool btvalidate(Oid opclassoid);
+extern void btcheckmembers(Oid opfamilyoid,
+                           Oid opclassoid,
+                           List *operators,
+                           List *functions);

 /*
  * prototypes for functions in nbtsort.c
diff --git a/src/include/access/spgist.h b/src/include/access/spgist.h
index f48080b..c95c67c 100644
--- a/src/include/access/spgist.h
+++ b/src/include/access/spgist.h
@@ -219,5 +219,9 @@ extern IndexBulkDeleteResult *spgvacuumcleanup(IndexVacuumInfo *info,

 /* spgvalidate.c */
 extern bool spgvalidate(Oid opclassoid);
+extern void spgcheckmembers(Oid opfamilyoid,
+                            Oid opclassoid,
+                            List *operators,
+                            List *functions);

 #endif                            /* SPGIST_H */
diff --git a/src/include/catalog/opfam_internal.h b/src/include/catalog/opfam_internal.h
deleted file mode 100644
index d63bd9f..0000000
--- a/src/include/catalog/opfam_internal.h
+++ /dev/null
@@ -1,28 +0,0 @@
-/*-------------------------------------------------------------------------
- *
- * opfam_internal.h
- *
- * Portions Copyright (c) 1996-2020, PostgreSQL Global Development Group
- * Portions Copyright (c) 1994, Regents of the University of California
- *
- * src/include/catalog/opfam_internal.h
- *
- *-------------------------------------------------------------------------
- */
-#ifndef OPFAM_INTERNAL_H
-#define OPFAM_INTERNAL_H
-
-/*
- * We use lists of this struct type to keep track of both operators and
- * procedures while building or adding to an opfamily.
- */
-typedef struct
-{
-    Oid            object;            /* operator or support proc's OID */
-    int            number;            /* strategy or support proc number */
-    Oid            lefttype;        /* lefttype */
-    Oid            righttype;        /* righttype */
-    Oid            sortfamily;        /* ordering operator's sort opfamily, or 0 */
-} OpFamilyMember;
-
-#endif                            /* OPFAM_INTERNAL_H */