Re: Rethinking opclass member checks and dependency strength

Alexander Korotkov <a.korotkov@postgrespro.ru> writes:
> On Sun, Aug 18, 2019 at 10:00 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> * Are the specific ways that the entries are getting set up appropriate?
>> Note in particular that I left btree/hash alone, feeling that the default
>> (historical) behavior was designed for them and is not unreasonable; but
>> maybe we should switch them to the cross-type-vs-not-cross-type behavior
>> proposed above.  Also I didn't touch BRIN because I don't know enough
>> about it to be sure what would be best, and I didn't touch contrib/bloom
>> because I don't care too much about it.

> I think we need ability to remove GiST fetch proc.  Presence of this
> procedure is used to determine whether GiST index supports index only
> scan (IOS).  We need to be able to remove this proc to drop IOS
> support.

OK ... so thinking in more general terms, you're arguing that any optional
support function should have a soft not hard dependency.  The attached v2
patch implements that rule, and also changes btree and hash to use
the cross-type-vs-not-cross-type behavior I proposed earlier.

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'm not at all impressed with the name, location, or concept of
>> opfam_internal.h.  I think we should get rid of that header and put
>> the OpFamilyMember struct somewhere else.  Given that this patch
>> makes it part of the AM API, it wouldn't be unreasonable to move it
>> to amapi.h.  But I've not done that here.

> +1

Did that in this revision, too.

            regards, tom lane

diff --git a/contrib/bloom/blutils.c b/contrib/bloom/blutils.c
index cc16709..5f664ca 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 ae7b729..129b6b5 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 cf9699a..d4f4851 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 63bd7f2..baf07a9 100644
--- a/src/backend/access/gin/ginvalidate.c
+++ b/src/backend/access/gin/ginvalidate.c
@@ -268,3 +268,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 0cc8791..e4cf0e8 100644
--- a/src/backend/access/gist/gist.c
+++ b/src/backend/access/gist/gist.c
@@ -88,6 +88,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 dfc1a87..8e2503f 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 5cc30da..3515a7e 100644
--- a/src/backend/access/hash/hash.c
+++ b/src/backend/access/hash/hash.c
@@ -87,6 +87,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 9315872..cbab4ec 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 5a27285..2045b72 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-2019, 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 4cfd528..14f2505 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -136,6 +136,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 0148ea7..468cd95 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 45472db..afb416e 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 4b9fdbd..1bdd352 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 6a1ccde..93a0ac2 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"
@@ -62,12 +61,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);
@@ -516,11 +513,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)
@@ -540,6 +538,7 @@ DefineOpClass(CreateOpClassStmt *stmt)

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

@@ -549,7 +548,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))
@@ -662,13 +661,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);
@@ -837,6 +868,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;
@@ -895,11 +927,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)
@@ -919,8 +957,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)
@@ -928,7 +972,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,
@@ -942,13 +986,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,
@@ -991,10 +1045,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)
@@ -1006,10 +1061,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 */
@@ -1264,7 +1320,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;

@@ -1276,7 +1332,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",
@@ -1298,13 +1354,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;
@@ -1374,28 +1427,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))
@@ -1403,8 +1445,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);
@@ -1416,13 +1461,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;
@@ -1486,28 +1528,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 4712e3a..1fc3fbf 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 6e3db06..2ee34d7 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 317e1e6..41e62e0 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-2019, 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 78fcd82..4ab44d6 100644
--- a/src/include/access/gin_private.h
+++ b/src/include/access/gin_private.h
@@ -388,6 +388,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 ed5b643..58c909e 100644
--- a/src/include/access/gist_private.h
+++ b/src/include/access/gist_private.h
@@ -469,6 +469,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 24af778..8287877 100644
--- a/src/include/access/hash.h
+++ b/src/include/access/hash.h
@@ -362,6 +362,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 4a80e84..3e5fe97 100644
--- a/src/include/access/nbtree.h
+++ b/src/include/access/nbtree.h
@@ -817,6 +817,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 d787ab2..2d1ae05 100644
--- a/src/include/access/spgist.h
+++ b/src/include/access/spgist.h
@@ -223,5 +223,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 9f17544..0000000
--- a/src/include/catalog/opfam_internal.h
+++ /dev/null
@@ -1,28 +0,0 @@
-/*-------------------------------------------------------------------------
- *
- * opfam_internal.h
- *
- * Portions Copyright (c) 1996-2019, 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 */