Improve index AMs' opclass validation procedures.
authorTom Lane <tgl@sss.pgh.pa.us>
Fri, 22 Jan 2016 00:47:15 +0000 (19:47 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Fri, 22 Jan 2016 00:47:15 +0000 (19:47 -0500)
The amvalidate functions added in commit 65c5fcd353a859da were on the
crude side.  Improve them in a few ways:

* Perform signature checking for operators and support functions.

* Apply more thorough checks for missing operators and functions,
where possible.

* Instead of reporting problems as ERRORs, report most problems as INFO
messages and make the amvalidate function return FALSE.  This allows
more than one problem to be discovered per run.

* Report object names rather than OIDs, and work a bit harder on making
the messages understandable.

Also, remove a few more opr_sanity regression test queries that are
now superseded by the amvalidate checks.

13 files changed:
src/backend/access/brin/brin_validate.c
src/backend/access/gin/ginvalidate.c
src/backend/access/gist/gistvalidate.c
src/backend/access/hash/hashvalidate.c
src/backend/access/index/Makefile
src/backend/access/index/amvalidate.c [new file with mode: 0644]
src/backend/access/nbtree/nbtvalidate.c
src/backend/access/spgist/spgvalidate.c
src/backend/utils/cache/lsyscache.c
src/include/access/amvalidate.h [new file with mode: 0644]
src/include/utils/lsyscache.h
src/test/regress/expected/opr_sanity.out
src/test/regress/sql/opr_sanity.sql

index 956ecb95a01e5d1604f2ae6975570f9e70a88265..4cb1bca8e45e4c0e5af79f94dc6365d9e8f1b4bc 100644 (file)
  */
 #include "postgres.h"
 
+#include "access/amvalidate.h"
 #include "access/brin_internal.h"
 #include "access/htup_details.h"
 #include "catalog/pg_amop.h"
 #include "catalog/pg_amproc.h"
 #include "catalog/pg_opclass.h"
-#include "utils/catcache.h"
+#include "catalog/pg_opfamily.h"
+#include "catalog/pg_type.h"
+#include "utils/builtins.h"
 #include "utils/syscache.h"
 
 
 /*
  * Validator for a BRIN opclass.
+ *
+ * Some of the checks done here cover the whole opfamily, and therefore are
+ * redundant when checking each opclass in a family.  But they don't run long
+ * enough to be much of a problem, so we accept the duplication rather than
+ * complicate the amvalidate API.
  */
 bool
 brinvalidate(Oid opclassoid)
 {
+   bool        result = true;
    HeapTuple   classtup;
    Form_pg_opclass classform;
    Oid         opfamilyoid;
    Oid         opcintype;
-   int         numclassops;
-   int32       classfuncbits;
+   char       *opclassname;
+   HeapTuple   familytup;
+   Form_pg_opfamily familyform;
+   char       *opfamilyname;
    CatCList   *proclist,
               *oprlist;
-   int         i,
-               j;
+   uint64      allfuncs = 0;
+   uint64      allops = 0;
+   List       *grouplist;
+   OpFamilyOpFuncGroup *opclassgroup;
+   int         i;
+   ListCell   *lc;
 
    /* Fetch opclass information */
    classtup = SearchSysCache1(CLAOID, ObjectIdGetDatum(opclassoid));
@@ -47,106 +62,217 @@ brinvalidate(Oid opclassoid)
 
    opfamilyoid = classform->opcfamily;
    opcintype = classform->opcintype;
+   opclassname = NameStr(classform->opcname);
 
-   ReleaseSysCache(classtup);
+   /* Fetch opfamily information */
+   familytup = SearchSysCache1(OPFAMILYOID, ObjectIdGetDatum(opfamilyoid));
+   if (!HeapTupleIsValid(familytup))
+       elog(ERROR, "cache lookup failed for operator family %u", opfamilyoid);
+   familyform = (Form_pg_opfamily) GETSTRUCT(familytup);
+
+   opfamilyname = NameStr(familyform->opfname);
 
    /* Fetch all operators and support functions of the opfamily */
    oprlist = SearchSysCacheList1(AMOPSTRATEGY, ObjectIdGetDatum(opfamilyoid));
    proclist = SearchSysCacheList1(AMPROCNUM, ObjectIdGetDatum(opfamilyoid));
 
-   /* We'll track the ops and functions belonging to the named opclass */
-   numclassops = 0;
-   classfuncbits = 0;
-
-   /* Check support functions */
+   /* Check individual support functions */
    for (i = 0; i < proclist->n_members; i++)
    {
        HeapTuple   proctup = &proclist->members[i]->tuple;
        Form_pg_amproc procform = (Form_pg_amproc) GETSTRUCT(proctup);
+       bool        ok;
 
-       /* Check that only allowed procedure numbers exist */
-       if (procform->amprocnum < 1 ||
-           procform->amprocnum > BRIN_LAST_OPTIONAL_PROCNUM)
-           ereport(ERROR,
+       /* Check procedure numbers and function signatures */
+       switch (procform->amprocnum)
+       {
+           case BRIN_PROCNUM_OPCINFO:
+               ok = check_amproc_signature(procform->amproc, INTERNALOID, true,
+                                           1, 1, INTERNALOID);
+               break;
+           case BRIN_PROCNUM_ADDVALUE:
+               ok = check_amproc_signature(procform->amproc, BOOLOID, true,
+                                           4, 4, INTERNALOID, INTERNALOID,
+                                           INTERNALOID, INTERNALOID);
+               break;
+           case BRIN_PROCNUM_CONSISTENT:
+               ok = check_amproc_signature(procform->amproc, BOOLOID, true,
+                                           3, 3, INTERNALOID, INTERNALOID,
+                                           INTERNALOID);
+               break;
+           case BRIN_PROCNUM_UNION:
+               ok = check_amproc_signature(procform->amproc, BOOLOID, true,
+                                           3, 3, INTERNALOID, INTERNALOID,
+                                           INTERNALOID);
+               break;
+           default:
+               /* Complain if it's not a valid optional proc number */
+               if (procform->amprocnum < BRIN_FIRST_OPTIONAL_PROCNUM ||
+                   procform->amprocnum > BRIN_LAST_OPTIONAL_PROCNUM)
+               {
+                   ereport(INFO,
+                           (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                            errmsg("brin opfamily %s contains function %s with invalid support number %d",
+                                   opfamilyname,
+                                   format_procedure(procform->amproc),
+                                   procform->amprocnum)));
+                   result = false;
+                   continue;   /* omit bad proc numbers from allfuncs */
+               }
+               /* Can't check signatures of optional procs, so assume OK */
+               ok = true;
+               break;
+       }
+
+       if (!ok)
+       {
+           ereport(INFO,
                    (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                    errmsg("brin opfamily %u contains invalid support number %d for procedure %u",
-                           opfamilyoid,
-                           procform->amprocnum, procform->amproc)));
-
-       /* Remember functions that are specifically for the named opclass */
-       if (procform->amproclefttype == opcintype &&
-           procform->amprocrighttype == opcintype)
-           classfuncbits |= (1 << procform->amprocnum);
+                    errmsg("brin opfamily %s contains function %s with wrong signature for support number %d",
+                           opfamilyname,
+                           format_procedure(procform->amproc),
+                           procform->amprocnum)));
+           result = false;
+       }
+
+       /* Track all valid procedure numbers seen in opfamily */
+       allfuncs |= ((uint64) 1) << procform->amprocnum;
    }
 
-   /* Check operators */
+   /* Check individual operators */
    for (i = 0; i < oprlist->n_members; i++)
    {
        HeapTuple   oprtup = &oprlist->members[i]->tuple;
        Form_pg_amop oprform = (Form_pg_amop) GETSTRUCT(oprtup);
-       bool        found = false;
 
-       /* TODO: Check that only allowed strategy numbers exist */
-       if (oprform->amopstrategy < 1)
-           ereport(ERROR,
+       /* Check that only allowed strategy numbers exist */
+       if (oprform->amopstrategy < 1 || oprform->amopstrategy > 63)
+       {
+           ereport(INFO,
                    (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                    errmsg("brin opfamily %u contains invalid strategy number %d for operator %u",
-                           opfamilyoid,
-                           oprform->amopstrategy, oprform->amopopr)));
-
-       /* TODO: check more thoroughly for missing support functions */
-       for (j = 0; j < proclist->n_members; j++)
+                    errmsg("brin opfamily %s contains operator %s with invalid strategy number %d",
+                           opfamilyname,
+                           format_operator(oprform->amopopr),
+                           oprform->amopstrategy)));
+           result = false;
+       }
+       else
        {
-           HeapTuple   proctup = &proclist->members[j]->tuple;
-           Form_pg_amproc procform = (Form_pg_amproc) GETSTRUCT(proctup);
-
-           /* note only the operator's lefttype matters */
-           if (procform->amproclefttype == oprform->amoplefttype &&
-               procform->amprocrighttype == oprform->amoplefttype)
-           {
-               found = true;
-               break;
-           }
+           /*
+            * The set of operators supplied varies across BRIN opfamilies.
+            * Our plan is to identify all operator strategy numbers used in
+            * the opfamily and then complain about datatype combinations that
+            * are missing any operator(s).  However, consider only numbers
+            * that appear in some non-cross-type case, since cross-type
+            * operators may have unique strategies.  (This is not a great
+            * heuristic, in particular an erroneous number used in a
+            * cross-type operator will not get noticed; but the core BRIN
+            * opfamilies are messy enough to make it necessary.)
+            */
+           if (oprform->amoplefttype == oprform->amoprighttype)
+               allops |= ((uint64) 1) << oprform->amopstrategy;
        }
 
-       if (!found)
-           ereport(ERROR,
-                   (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-           errmsg("brin opfamily %u lacks support function for operator %u",
-                  opfamilyoid, oprform->amopopr)));
-
        /* brin doesn't support ORDER BY operators */
        if (oprform->amoppurpose != AMOP_SEARCH ||
            OidIsValid(oprform->amopsortfamily))
-           ereport(ERROR,
+       {
+           ereport(INFO,
+                   (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                    errmsg("brin opfamily %s contains invalid ORDER BY specification for operator %s",
+                           opfamilyname,
+                           format_operator(oprform->amopopr))));
+           result = false;
+       }
+
+       /* Check operator signature --- same for all brin strategies */
+       if (!check_amop_signature(oprform->amopopr, BOOLOID,
+                                 oprform->amoplefttype,
+                                 oprform->amoprighttype))
+       {
+           ereport(INFO,
                    (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                    errmsg("brin opfamily %u contains invalid ORDER BY specification for operator %u",
-                           opfamilyoid, oprform->amopopr)));
+                    errmsg("brin opfamily %s contains operator %s with wrong signature",
+                           opfamilyname,
+                           format_operator(oprform->amopopr))));
+           result = false;
+       }
+   }
+
+   /* Now check for inconsistent groups of operators/functions */
+   grouplist = identify_opfamily_groups(oprlist, proclist);
+   opclassgroup = NULL;
+   foreach(lc, grouplist)
+   {
+       OpFamilyOpFuncGroup *thisgroup = (OpFamilyOpFuncGroup *) lfirst(lc);
+
+       /* Remember the group exactly matching the test opclass */
+       if (thisgroup->lefttype == opcintype &&
+           thisgroup->righttype == opcintype)
+           opclassgroup = thisgroup;
 
-       /* Count operators that are specifically for the named opclass */
-       if (oprform->amoplefttype == opcintype &&
-           oprform->amoprighttype == opcintype)
-           numclassops++;
+       /*
+        * Some BRIN opfamilies expect cross-type support functions to exist,
+        * and some don't.  We don't know exactly which are which, so if we
+        * find a cross-type operator for which there are no support functions
+        * at all, let it pass.  (Don't expect that all operators exist for
+        * such cross-type cases, either.)
+        */
+       if (thisgroup->functionset == 0 &&
+           thisgroup->lefttype != thisgroup->righttype)
+           continue;
+
+       /*
+        * Else complain if there seems to be an incomplete set of either
+        * operators or support functions for this datatype pair.
+        */
+       if (thisgroup->operatorset != allops)
+       {
+           ereport(INFO,
+                   (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                    errmsg("brin opfamily %s is missing operator(s) for types %s and %s",
+                           opfamilyname,
+                           format_type_be(thisgroup->lefttype),
+                           format_type_be(thisgroup->righttype))));
+           result = false;
+       }
+       if (thisgroup->functionset != allfuncs)
+       {
+           ereport(INFO,
+                   (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                    errmsg("brin opfamily %s is missing support function(s) for types %s and %s",
+                           opfamilyname,
+                           format_type_be(thisgroup->lefttype),
+                           format_type_be(thisgroup->righttype))));
+           result = false;
+       }
    }
 
-   /* Check that the named opclass is complete */
-   if (numclassops == 0)
-       ereport(ERROR,
+   /* Check that the originally-named opclass is complete */
+   if (!opclassgroup || opclassgroup->operatorset != allops)
+   {
+       ereport(INFO,
                (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                errmsg("brin opclass %u is missing operator(s)",
-                       opclassoid)));
+                errmsg("brin opclass %s is missing operator(s)",
+                       opclassname)));
+       result = false;
+   }
    for (i = 1; i <= BRIN_MANDATORY_NPROCS; i++)
    {
-       if ((classfuncbits & (1 << i)) != 0)
+       if (opclassgroup &&
+           (opclassgroup->functionset & (((int64) 1) << i)) != 0)
            continue;           /* got it */
-       ereport(ERROR,
+       ereport(INFO,
                (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-           errmsg("brin opclass %u is missing required support function %d",
-                  opclassoid, i)));
+                errmsg("brin opclass %s is missing support function %d",
+                       opclassname, i)));
+       result = false;
    }
 
    ReleaseCatCacheList(proclist);
    ReleaseCatCacheList(oprlist);
+   ReleaseSysCache(familytup);
+   ReleaseSysCache(classtup);
 
-   return true;
+   return result;
 }
index e934fe824b565941c989800603e7b14362aefed5..b87833bd5a10b0c4790cabcce17e78dd86a0e06c 100644 (file)
  */
 #include "postgres.h"
 
+#include "access/amvalidate.h"
 #include "access/gin_private.h"
 #include "access/htup_details.h"
 #include "catalog/pg_amop.h"
 #include "catalog/pg_amproc.h"
 #include "catalog/pg_opclass.h"
-#include "utils/catcache.h"
+#include "catalog/pg_opfamily.h"
+#include "catalog/pg_type.h"
+#include "utils/builtins.h"
+#include "utils/lsyscache.h"
 #include "utils/syscache.h"
 
 
 bool
 ginvalidate(Oid opclassoid)
 {
+   bool        result = true;
    HeapTuple   classtup;
    Form_pg_opclass classform;
    Oid         opfamilyoid;
    Oid         opcintype;
-   int         numclassops;
-   int32       classfuncbits;
+   Oid         opckeytype;
+   char       *opclassname;
+   HeapTuple   familytup;
+   Form_pg_opfamily familyform;
+   char       *opfamilyname;
    CatCList   *proclist,
               *oprlist;
+   List       *grouplist;
+   OpFamilyOpFuncGroup *opclassgroup;
    int         i;
+   ListCell   *lc;
 
    /* Fetch opclass information */
    classtup = SearchSysCache1(CLAOID, ObjectIdGetDatum(opclassoid));
@@ -46,100 +57,212 @@ ginvalidate(Oid opclassoid)
 
    opfamilyoid = classform->opcfamily;
    opcintype = classform->opcintype;
+   opckeytype = classform->opckeytype;
+   if (!OidIsValid(opckeytype))
+       opckeytype = opcintype;
+   opclassname = NameStr(classform->opcname);
 
-   ReleaseSysCache(classtup);
+   /* Fetch opfamily information */
+   familytup = SearchSysCache1(OPFAMILYOID, ObjectIdGetDatum(opfamilyoid));
+   if (!HeapTupleIsValid(familytup))
+       elog(ERROR, "cache lookup failed for operator family %u", opfamilyoid);
+   familyform = (Form_pg_opfamily) GETSTRUCT(familytup);
+
+   opfamilyname = NameStr(familyform->opfname);
 
    /* Fetch all operators and support functions of the opfamily */
    oprlist = SearchSysCacheList1(AMOPSTRATEGY, ObjectIdGetDatum(opfamilyoid));
    proclist = SearchSysCacheList1(AMPROCNUM, ObjectIdGetDatum(opfamilyoid));
 
-   /* We'll track the ops and functions belonging to the named opclass */
-   numclassops = 0;
-   classfuncbits = 0;
-
-   /* Check support functions */
+   /* Check individual support functions */
    for (i = 0; i < proclist->n_members; i++)
    {
        HeapTuple   proctup = &proclist->members[i]->tuple;
        Form_pg_amproc procform = (Form_pg_amproc) GETSTRUCT(proctup);
+       bool        ok;
+
+       /*
+        * All GIN support functions should be registered with matching
+        * left/right types
+        */
+       if (procform->amproclefttype != procform->amprocrighttype)
+       {
+           ereport(INFO,
+                   (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                    errmsg("gin opfamily %s contains support procedure %s with cross-type registration",
+                           opfamilyname,
+                           format_procedure(procform->amproc))));
+           result = false;
+       }
+
+       /*
+        * We can't check signatures except within the specific opclass, since
+        * we need to know the associated opckeytype in many cases.
+        */
+       if (procform->amproclefttype != opcintype)
+           continue;
 
-       /* Check that only allowed procedure numbers exist */
-       if (procform->amprocnum < 1 ||
-           procform->amprocnum > GINNProcs)
-           ereport(ERROR,
+       /* Check procedure numbers and function signatures */
+       switch (procform->amprocnum)
+       {
+           case GIN_COMPARE_PROC:
+               ok = check_amproc_signature(procform->amproc, INT4OID, false,
+                                           2, 2, opckeytype, opckeytype);
+               break;
+           case GIN_EXTRACTVALUE_PROC:
+               /* Some opclasses omit nullFlags */
+               ok = check_amproc_signature(procform->amproc, INTERNALOID, false,
+                                           2, 3, opcintype, INTERNALOID,
+                                           INTERNALOID);
+               break;
+           case GIN_EXTRACTQUERY_PROC:
+               /* Some opclasses omit nullFlags and searchMode */
+               ok = check_amproc_signature(procform->amproc, INTERNALOID, false,
+                                           5, 7, opcintype, INTERNALOID,
+                                           INT2OID, INTERNALOID, INTERNALOID,
+                                           INTERNALOID, INTERNALOID);
+               break;
+           case GIN_CONSISTENT_PROC:
+               /* Some opclasses omit queryKeys and nullFlags */
+               ok = check_amproc_signature(procform->amproc, BOOLOID, false,
+                                           6, 8, INTERNALOID, INT2OID,
+                                           opcintype, INT4OID,
+                                           INTERNALOID, INTERNALOID,
+                                           INTERNALOID, INTERNALOID);
+               break;
+           case GIN_COMPARE_PARTIAL_PROC:
+               ok = check_amproc_signature(procform->amproc, INT4OID, false,
+                                           4, 4, opckeytype, opckeytype,
+                                           INT2OID, INTERNALOID);
+               break;
+           case GIN_TRICONSISTENT_PROC:
+               ok = check_amproc_signature(procform->amproc, CHAROID, false,
+                                           7, 7, INTERNALOID, INT2OID,
+                                           opcintype, INT4OID,
+                                           INTERNALOID, INTERNALOID,
+                                           INTERNALOID);
+               break;
+           default:
+               ereport(INFO,
+                       (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                        errmsg("gin opfamily %s contains function %s with invalid support number %d",
+                               opfamilyname,
+                               format_procedure(procform->amproc),
+                               procform->amprocnum)));
+               result = false;
+               continue;       /* don't want additional message */
+       }
+
+       if (!ok)
+       {
+           ereport(INFO,
                    (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                    errmsg("gin opfamily %u contains invalid support number %d for procedure %u",
-                           opfamilyoid,
-                           procform->amprocnum, procform->amproc)));
-
-       /* Remember functions that are specifically for the named opclass */
-       if (procform->amproclefttype == opcintype &&
-           procform->amprocrighttype == opcintype)
-           classfuncbits |= (1 << procform->amprocnum);
+                    errmsg("gin opfamily %s contains function %s with wrong signature for support number %d",
+                           opfamilyname,
+                           format_procedure(procform->amproc),
+                           procform->amprocnum)));
+           result = false;
+       }
    }
 
-   /* Check operators */
+   /* Check individual operators */
    for (i = 0; i < oprlist->n_members; i++)
    {
        HeapTuple   oprtup = &oprlist->members[i]->tuple;
        Form_pg_amop oprform = (Form_pg_amop) GETSTRUCT(oprtup);
 
        /* TODO: Check that only allowed strategy numbers exist */
-       if (oprform->amopstrategy < 1)
-           ereport(ERROR,
+       if (oprform->amopstrategy < 1 || oprform->amopstrategy > 63)
+       {
+           ereport(INFO,
                    (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                    errmsg("gin opfamily %u contains invalid strategy number %d for operator %u",
-                           opfamilyoid,
-                           oprform->amopstrategy, oprform->amopopr)));
+                    errmsg("gin opfamily %s contains operator %s with invalid strategy number %d",
+                           opfamilyname,
+                           format_operator(oprform->amopopr),
+                           oprform->amopstrategy)));
+           result = false;
+       }
 
        /* gin doesn't support ORDER BY operators */
        if (oprform->amoppurpose != AMOP_SEARCH ||
            OidIsValid(oprform->amopsortfamily))
-           ereport(ERROR,
+       {
+           ereport(INFO,
                    (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                    errmsg("gin opfamily %u contains invalid ORDER BY specification for operator %u",
-                           opfamilyoid, oprform->amopopr)));
+                    errmsg("gin opfamily %s contains invalid ORDER BY specification for operator %s",
+                           opfamilyname,
+                           format_operator(oprform->amopopr))));
+           result = false;
+       }
 
-       /* Count operators that are specifically for the named opclass */
-       if (oprform->amoplefttype == opcintype &&
-           oprform->amoprighttype == opcintype)
-           numclassops++;
+       /* Check operator signature --- same for all gin strategies */
+       if (!check_amop_signature(oprform->amopopr, BOOLOID,
+                                 oprform->amoplefttype,
+                                 oprform->amoprighttype))
+       {
+           ereport(INFO,
+                   (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                    errmsg("gin opfamily %s contains operator %s with wrong signature",
+                           opfamilyname,
+                           format_operator(oprform->amopopr))));
+           result = false;
+       }
    }
 
-   /* Check that the named opclass is complete */
+   /* Now check for inconsistent groups of operators/functions */
+   grouplist = identify_opfamily_groups(oprlist, proclist);
+   opclassgroup = NULL;
+   foreach(lc, grouplist)
+   {
+       OpFamilyOpFuncGroup *thisgroup = (OpFamilyOpFuncGroup *) lfirst(lc);
 
-   /* XXX needs work: we need to detect applicability of ANYARRAY operators */
-#ifdef NOT_USED
-   if (numclassops == 0)
-       ereport(ERROR,
-               (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                errmsg("gin opclass %u is missing operator(s)",
-                       opclassoid)));
-#endif
+       /* Remember the group exactly matching the test opclass */
+       if (thisgroup->lefttype == opcintype &&
+           thisgroup->righttype == opcintype)
+           opclassgroup = thisgroup;
+
+       /*
+        * There is not a lot we can do to check the operator sets, since each
+        * GIN opclass is more or less a law unto itself, and some contain
+        * only operators that are binary-compatible with the opclass datatype
+        * (meaning that empty operator sets can be OK).  That case also means
+        * that we shouldn't insist on nonempty function sets except for the
+        * opclass's own group.
+        */
+   }
 
+   /* Check that the originally-named opclass is complete */
    for (i = 1; i <= GINNProcs; i++)
    {
-       if ((classfuncbits & (1 << i)) != 0)
+       if (opclassgroup && (opclassgroup->functionset & (1 << i)) != 0)
            continue;           /* got it */
        if (i == GIN_COMPARE_PARTIAL_PROC)
            continue;           /* optional method */
        if (i == GIN_CONSISTENT_PROC || i == GIN_TRICONSISTENT_PROC)
-           continue;           /* don't need to have both, see check below
-                                * loop */
-       ereport(ERROR,
+           continue;           /* don't need both, see check below loop */
+       ereport(INFO,
                (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-            errmsg("gin opclass %u is missing required support function %d",
-                   opclassoid, i)));
+                errmsg("gin opclass %s is missing support function %d",
+                       opclassname, i)));
+       result = false;
    }
-   if ((classfuncbits & (1 << GIN_CONSISTENT_PROC)) == 0 &&
-       (classfuncbits & (1 << GIN_TRICONSISTENT_PROC)) == 0)
-       ereport(ERROR,
+   if (!opclassgroup ||
+       ((opclassgroup->functionset & (1 << GIN_CONSISTENT_PROC)) == 0 &&
+        (opclassgroup->functionset & (1 << GIN_TRICONSISTENT_PROC)) == 0))
+   {
+       ereport(INFO,
                (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                errmsg("gin opclass %u is missing required support function",
-                       opclassoid)));
+                errmsg("gin opclass %s is missing support function %d or %d",
+                       opclassname,
+                       GIN_CONSISTENT_PROC, GIN_TRICONSISTENT_PROC)));
+       result = false;
+   }
+
 
    ReleaseCatCacheList(proclist);
    ReleaseCatCacheList(oprlist);
+   ReleaseSysCache(familytup);
+   ReleaseSysCache(classtup);
 
-   return true;
+   return result;
 }
index 86b5aeaec5c29e417f90dae08b7bd7c2c54590a9..190b9787bb7040c39e9f7749f9245381c23a1e53 100644 (file)
  */
 #include "postgres.h"
 
+#include "access/amvalidate.h"
 #include "access/gist_private.h"
 #include "access/htup_details.h"
 #include "catalog/pg_amop.h"
 #include "catalog/pg_amproc.h"
 #include "catalog/pg_opclass.h"
-#include "utils/catcache.h"
+#include "catalog/pg_opfamily.h"
+#include "catalog/pg_type.h"
+#include "utils/builtins.h"
+#include "utils/lsyscache.h"
 #include "utils/syscache.h"
 
 
 bool
 gistvalidate(Oid opclassoid)
 {
+   bool        result = true;
    HeapTuple   classtup;
    Form_pg_opclass classform;
    Oid         opfamilyoid;
    Oid         opcintype;
-   int         numclassops;
-   int32       classfuncbits;
+   Oid         opckeytype;
+   char       *opclassname;
+   HeapTuple   familytup;
+   Form_pg_opfamily familyform;
+   char       *opfamilyname;
    CatCList   *proclist,
               *oprlist;
+   List       *grouplist;
+   OpFamilyOpFuncGroup *opclassgroup;
    int         i;
+   ListCell   *lc;
 
    /* Fetch opclass information */
    classtup = SearchSysCache1(CLAOID, ObjectIdGetDatum(opclassoid));
@@ -46,88 +57,218 @@ gistvalidate(Oid opclassoid)
 
    opfamilyoid = classform->opcfamily;
    opcintype = classform->opcintype;
+   opckeytype = classform->opckeytype;
+   if (!OidIsValid(opckeytype))
+       opckeytype = opcintype;
+   opclassname = NameStr(classform->opcname);
 
-   ReleaseSysCache(classtup);
+   /* Fetch opfamily information */
+   familytup = SearchSysCache1(OPFAMILYOID, ObjectIdGetDatum(opfamilyoid));
+   if (!HeapTupleIsValid(familytup))
+       elog(ERROR, "cache lookup failed for operator family %u", opfamilyoid);
+   familyform = (Form_pg_opfamily) GETSTRUCT(familytup);
+
+   opfamilyname = NameStr(familyform->opfname);
 
    /* Fetch all operators and support functions of the opfamily */
    oprlist = SearchSysCacheList1(AMOPSTRATEGY, ObjectIdGetDatum(opfamilyoid));
    proclist = SearchSysCacheList1(AMPROCNUM, ObjectIdGetDatum(opfamilyoid));
 
-   /* We'll track the ops and functions belonging to the named opclass */
-   numclassops = 0;
-   classfuncbits = 0;
-
-   /* Check support functions */
+   /* Check individual support functions */
    for (i = 0; i < proclist->n_members; i++)
    {
        HeapTuple   proctup = &proclist->members[i]->tuple;
        Form_pg_amproc procform = (Form_pg_amproc) GETSTRUCT(proctup);
+       bool        ok;
+
+       /*
+        * All GiST support functions should be registered with matching
+        * left/right types
+        */
+       if (procform->amproclefttype != procform->amprocrighttype)
+       {
+           ereport(INFO,
+                   (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                    errmsg("gist opfamily %s contains support procedure %s with cross-type registration",
+                           opfamilyname,
+                           format_procedure(procform->amproc))));
+           result = false;
+       }
+
+       /*
+        * We can't check signatures except within the specific opclass, since
+        * we need to know the associated opckeytype in many cases.
+        */
+       if (procform->amproclefttype != opcintype)
+           continue;
 
-       /* Check that only allowed procedure numbers exist */
-       if (procform->amprocnum < 1 ||
-           procform->amprocnum > GISTNProcs)
-           ereport(ERROR,
+       /* Check procedure numbers and function signatures */
+       switch (procform->amprocnum)
+       {
+           case GIST_CONSISTENT_PROC:
+               ok = check_amproc_signature(procform->amproc, BOOLOID, false,
+                                           5, 5, INTERNALOID, opcintype,
+                                           INT2OID, OIDOID, INTERNALOID);
+               break;
+           case GIST_UNION_PROC:
+               ok = check_amproc_signature(procform->amproc, opckeytype, false,
+                                           2, 2, INTERNALOID, INTERNALOID);
+               break;
+           case GIST_COMPRESS_PROC:
+           case GIST_DECOMPRESS_PROC:
+           case GIST_FETCH_PROC:
+               ok = check_amproc_signature(procform->amproc, INTERNALOID, true,
+                                           1, 1, INTERNALOID);
+               break;
+           case GIST_PENALTY_PROC:
+               ok = check_amproc_signature(procform->amproc, INTERNALOID, true,
+                                           3, 3, INTERNALOID,
+                                           INTERNALOID, INTERNALOID);
+               break;
+           case GIST_PICKSPLIT_PROC:
+               ok = check_amproc_signature(procform->amproc, INTERNALOID, true,
+                                           2, 2, INTERNALOID, INTERNALOID);
+               break;
+           case GIST_EQUAL_PROC:
+               ok = check_amproc_signature(procform->amproc, INTERNALOID, false,
+                                           3, 3, opckeytype, opckeytype,
+                                           INTERNALOID);
+               break;
+           case GIST_DISTANCE_PROC:
+               ok = check_amproc_signature(procform->amproc, FLOAT8OID, false,
+                                           5, 5, INTERNALOID, opcintype,
+                                           INT2OID, OIDOID, INTERNALOID);
+               break;
+           default:
+               ereport(INFO,
+                       (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                        errmsg("gist opfamily %s contains function %s with invalid support number %d",
+                               opfamilyname,
+                               format_procedure(procform->amproc),
+                               procform->amprocnum)));
+               result = false;
+               continue;       /* don't want additional message */
+       }
+
+       if (!ok)
+       {
+           ereport(INFO,
                    (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                    errmsg("gist opfamily %u contains invalid support number %d for procedure %u",
-                           opfamilyoid,
-                           procform->amprocnum, procform->amproc)));
-
-       /* Remember functions that are specifically for the named opclass */
-       if (procform->amproclefttype == opcintype &&
-           procform->amprocrighttype == opcintype)
-           classfuncbits |= (1 << procform->amprocnum);
+                    errmsg("gist opfamily %s contains function %s with wrong signature for support number %d",
+                           opfamilyname,
+                           format_procedure(procform->amproc),
+                           procform->amprocnum)));
+           result = false;
+       }
    }
 
-   /* Check operators */
+   /* Check individual operators */
    for (i = 0; i < oprlist->n_members; i++)
    {
        HeapTuple   oprtup = &oprlist->members[i]->tuple;
        Form_pg_amop oprform = (Form_pg_amop) GETSTRUCT(oprtup);
+       Oid         op_rettype;
 
        /* TODO: Check that only allowed strategy numbers exist */
        if (oprform->amopstrategy < 1)
-           ereport(ERROR,
+       {
+           ereport(INFO,
                    (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                    errmsg("gist opfamily %u contains invalid strategy number %d for operator %u",
-                           opfamilyoid,
-                           oprform->amopstrategy, oprform->amopopr)));
-
-       /* GiST supports ORDER BY operators, but must have distance proc */
-       if (oprform->amoppurpose != AMOP_SEARCH &&
-           oprform->amoplefttype == opcintype &&
-           oprform->amoprighttype == opcintype &&
-           (classfuncbits & (1 << GIST_DISTANCE_PROC)) == 0)
-           ereport(ERROR,
+                    errmsg("gist opfamily %s contains operator %s with invalid strategy number %d",
+                           opfamilyname,
+                           format_operator(oprform->amopopr),
+                           oprform->amopstrategy)));
+           result = false;
+       }
+
+       /* GiST supports ORDER BY operators */
+       if (oprform->amoppurpose != AMOP_SEARCH)
+       {
+           /* ... but must have matching distance proc */
+           if (!OidIsValid(get_opfamily_proc(opfamilyoid,
+                                             oprform->amoplefttype,
+                                             oprform->amoplefttype,
+                                             GIST_DISTANCE_PROC)))
+           {
+               ereport(INFO,
+                       (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                        errmsg("gist opfamily %s contains unsupported ORDER BY specification for operator %s",
+                               opfamilyname,
+                               format_operator(oprform->amopopr))));
+               result = false;
+           }
+           /* ... and operator result must match the claimed btree opfamily */
+           op_rettype = get_op_rettype(oprform->amopopr);
+           if (!opfamily_can_sort_type(oprform->amopsortfamily, op_rettype))
+           {
+               ereport(INFO,
+                       (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                        errmsg("gist opfamily %s contains incorrect ORDER BY opfamily specification for operator %s",
+                               opfamilyname,
+                               format_operator(oprform->amopopr))));
+               result = false;
+           }
+       }
+       else
+       {
+           /* Search operators must always return bool */
+           op_rettype = BOOLOID;
+       }
+
+       /* Check operator signature */
+       if (!check_amop_signature(oprform->amopopr, op_rettype,
+                                 oprform->amoplefttype,
+                                 oprform->amoprighttype))
+       {
+           ereport(INFO,
                    (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                    errmsg("gist opfamily %u contains unsupported ORDER BY specification for operator %u",
-                           opfamilyoid, oprform->amopopr)));
+                    errmsg("gist opfamily %s contains operator %s with wrong signature",
+                           opfamilyname,
+                           format_operator(oprform->amopopr))));
+           result = false;
+       }
+   }
+
+   /* Now check for inconsistent groups of operators/functions */
+   grouplist = identify_opfamily_groups(oprlist, proclist);
+   opclassgroup = NULL;
+   foreach(lc, grouplist)
+   {
+       OpFamilyOpFuncGroup *thisgroup = (OpFamilyOpFuncGroup *) lfirst(lc);
 
-       /* Count operators that are specifically for the named opclass */
-       /* XXX we consider only lefttype here */
-       if (oprform->amoplefttype == opcintype)
-           numclassops++;
+       /* Remember the group exactly matching the test opclass */
+       if (thisgroup->lefttype == opcintype &&
+           thisgroup->righttype == opcintype)
+           opclassgroup = thisgroup;
+
+       /*
+        * There is not a lot we can do to check the operator sets, since each
+        * GiST opclass is more or less a law unto itself, and some contain
+        * only operators that are binary-compatible with the opclass datatype
+        * (meaning that empty operator sets can be OK).  That case also means
+        * that we shouldn't insist on nonempty function sets except for the
+        * opclass's own group.
+        */
    }
 
-   /* Check that the named opclass is complete */
-   if (numclassops == 0)
-       ereport(ERROR,
-               (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                errmsg("gist opclass %u is missing operator(s)",
-                       opclassoid)));
+   /* Check that the originally-named opclass is complete */
    for (i = 1; i <= GISTNProcs; i++)
    {
-       if ((classfuncbits & (1 << i)) != 0)
+       if (opclassgroup && (opclassgroup->functionset & (1 << i)) != 0)
            continue;           /* got it */
        if (i == GIST_DISTANCE_PROC || i == GIST_FETCH_PROC)
            continue;           /* optional methods */
-       ereport(ERROR,
+       ereport(INFO,
                (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-           errmsg("gist opclass %u is missing required support function %d",
-                  opclassoid, i)));
+                errmsg("gist opclass %s is missing support function %d",
+                       opclassname, i)));
+       result = false;
    }
 
    ReleaseCatCacheList(proclist);
    ReleaseCatCacheList(oprlist);
+   ReleaseSysCache(familytup);
+   ReleaseSysCache(classtup);
 
-   return true;
+   return result;
 }
index abd678483c80a07fbc154bc97f73f8c0c655462a..f0395c5ce5f20046cfbca3ccb9c19ac5dc79243a 100644 (file)
  */
 #include "postgres.h"
 
+#include "access/amvalidate.h"
 #include "access/hash.h"
 #include "access/htup_details.h"
 #include "catalog/pg_amop.h"
 #include "catalog/pg_amproc.h"
 #include "catalog/pg_opclass.h"
+#include "catalog/pg_opfamily.h"
+#include "catalog/pg_proc.h"
+#include "catalog/pg_type.h"
+#include "parser/parse_coerce.h"
 #include "utils/builtins.h"
-#include "utils/catcache.h"
+#include "utils/fmgroids.h"
 #include "utils/syscache.h"
 
 
+static bool check_hash_func_signature(Oid funcid, Oid restype, Oid argtype);
+
+
 /*
  * Validator for a hash opclass.
  *
  * redundant when checking each opclass in a family.  But they don't run long
  * enough to be much of a problem, so we accept the duplication rather than
  * complicate the amvalidate API.
- *
- * Some of the code here relies on the fact that hash has only one operator
- * strategy and support function; we don't have to check for incomplete sets.
  */
 bool
 hashvalidate(Oid opclassoid)
 {
+   bool        result = true;
    HeapTuple   classtup;
    Form_pg_opclass classform;
    Oid         opfamilyoid;
    Oid         opcintype;
-   int         numclassops;
-   int32       classfuncbits;
+   char       *opclassname;
+   HeapTuple   familytup;
+   Form_pg_opfamily familyform;
+   char       *opfamilyname;
    CatCList   *proclist,
               *oprlist;
-   int         i,
-               j;
+   List       *grouplist;
+   OpFamilyOpFuncGroup *opclassgroup;
+   List       *hashabletypes = NIL;
+   int         i;
+   ListCell   *lc;
 
    /* Fetch opclass information */
    classtup = SearchSysCache1(CLAOID, ObjectIdGetDatum(opclassoid));
@@ -56,102 +67,246 @@ hashvalidate(Oid opclassoid)
 
    opfamilyoid = classform->opcfamily;
    opcintype = classform->opcintype;
+   opclassname = NameStr(classform->opcname);
 
-   ReleaseSysCache(classtup);
+   /* Fetch opfamily information */
+   familytup = SearchSysCache1(OPFAMILYOID, ObjectIdGetDatum(opfamilyoid));
+   if (!HeapTupleIsValid(familytup))
+       elog(ERROR, "cache lookup failed for operator family %u", opfamilyoid);
+   familyform = (Form_pg_opfamily) GETSTRUCT(familytup);
+
+   opfamilyname = NameStr(familyform->opfname);
 
    /* Fetch all operators and support functions of the opfamily */
    oprlist = SearchSysCacheList1(AMOPSTRATEGY, ObjectIdGetDatum(opfamilyoid));
    proclist = SearchSysCacheList1(AMPROCNUM, ObjectIdGetDatum(opfamilyoid));
 
-   /* We'll track the ops and functions belonging to the named opclass */
-   numclassops = 0;
-   classfuncbits = 0;
-
-   /* Check support functions */
+   /* Check individual support functions */
    for (i = 0; i < proclist->n_members; i++)
    {
        HeapTuple   proctup = &proclist->members[i]->tuple;
        Form_pg_amproc procform = (Form_pg_amproc) GETSTRUCT(proctup);
 
-       /* Check that only allowed procedure numbers exist */
-       if (procform->amprocnum != HASHPROC)
-           ereport(ERROR,
+       /*
+        * All hash functions should be registered with matching left/right
+        * types
+        */
+       if (procform->amproclefttype != procform->amprocrighttype)
+       {
+           ereport(INFO,
                    (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                    errmsg("hash opfamily %u contains invalid support number %d for procedure %u",
-                           opfamilyoid,
-                           procform->amprocnum, procform->amproc)));
-
-       /* Remember functions that are specifically for the named opclass */
-       if (procform->amproclefttype == opcintype &&
-           procform->amprocrighttype == opcintype)
-           classfuncbits |= (1 << procform->amprocnum);
+                    errmsg("hash opfamily %s contains support procedure %s with cross-type registration",
+                           opfamilyname,
+                           format_procedure(procform->amproc))));
+           result = false;
+       }
+
+       /* Check procedure numbers and function signatures */
+       switch (procform->amprocnum)
+       {
+           case HASHPROC:
+               if (!check_hash_func_signature(procform->amproc, INT4OID,
+                                              procform->amproclefttype))
+               {
+                   ereport(INFO,
+                           (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                            errmsg("hash opfamily %s contains function %s with wrong signature for support number %d",
+                                   opfamilyname,
+                                   format_procedure(procform->amproc),
+                                   procform->amprocnum)));
+                   result = false;
+               }
+               else
+               {
+                   /* Remember which types we can hash */
+                   hashabletypes =
+                       list_append_unique_oid(hashabletypes,
+                                              procform->amproclefttype);
+               }
+               break;
+           default:
+               ereport(INFO,
+                       (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                        errmsg("hash opfamily %s contains function %s with invalid support number %d",
+                               opfamilyname,
+                               format_procedure(procform->amproc),
+                               procform->amprocnum)));
+               result = false;
+               break;
+       }
    }
 
-   /* Check operators */
+   /* Check individual operators */
    for (i = 0; i < oprlist->n_members; i++)
    {
        HeapTuple   oprtup = &oprlist->members[i]->tuple;
        Form_pg_amop oprform = (Form_pg_amop) GETSTRUCT(oprtup);
-       bool        leftFound = false,
-                   rightFound = false;
 
        /* Check that only allowed strategy numbers exist */
        if (oprform->amopstrategy < 1 ||
            oprform->amopstrategy > HTMaxStrategyNumber)
-           ereport(ERROR,
+       {
+           ereport(INFO,
                    (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                    errmsg("hash opfamily %u contains invalid strategy number %d for operator %u",
-                           opfamilyoid,
-                           oprform->amopstrategy, oprform->amopopr)));
+                    errmsg("hash opfamily %s contains operator %s with invalid strategy number %d",
+                           opfamilyname,
+                           format_operator(oprform->amopopr),
+                           oprform->amopstrategy)));
+           result = false;
+       }
 
-       /*
-        * There should be relevant hash procedures for each operator
-        */
-       for (j = 0; j < proclist->n_members; j++)
+       /* hash doesn't support ORDER BY operators */
+       if (oprform->amoppurpose != AMOP_SEARCH ||
+           OidIsValid(oprform->amopsortfamily))
        {
-           HeapTuple   proctup = &proclist->members[j]->tuple;
-           Form_pg_amproc procform = (Form_pg_amproc) GETSTRUCT(proctup);
-
-           if (procform->amproclefttype == oprform->amoplefttype)
-               leftFound = true;
-           if (procform->amproclefttype == oprform->amoprighttype)
-               rightFound = true;
+           ereport(INFO,
+                   (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                    errmsg("hash opfamily %s contains invalid ORDER BY specification for operator %s",
+                           opfamilyname,
+                           format_operator(oprform->amopopr))));
+           result = false;
        }
 
-       if (!leftFound || !rightFound)
-           ereport(ERROR,
+       /* Check operator signature --- same for all hash strategies */
+       if (!check_amop_signature(oprform->amopopr, BOOLOID,
+                                 oprform->amoplefttype,
+                                 oprform->amoprighttype))
+       {
+           ereport(INFO,
                    (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-           errmsg("hash opfamily %u lacks support function for operator %u",
-                  opfamilyoid, oprform->amopopr)));
+                    errmsg("hash opfamily %s contains operator %s with wrong signature",
+                           opfamilyname,
+                           format_operator(oprform->amopopr))));
+           result = false;
+       }
 
-       /* hash doesn't support ORDER BY operators */
-       if (oprform->amoppurpose != AMOP_SEARCH ||
-           OidIsValid(oprform->amopsortfamily))
-           ereport(ERROR,
+       /* There should be relevant hash procedures for each datatype */
+       if (!list_member_oid(hashabletypes, oprform->amoplefttype) ||
+           !list_member_oid(hashabletypes, oprform->amoprighttype))
+       {
+           ereport(INFO,
                    (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                    errmsg("hash opfamily %u contains invalid ORDER BY specification for operator %u",
-                           opfamilyoid, oprform->amopopr)));
+           errmsg("hash opfamily %s lacks support function for operator %s",
+                  opfamilyname,
+                  format_operator(oprform->amopopr))));
+           result = false;
+       }
+   }
+
+   /* Now check for inconsistent groups of operators/functions */
+   grouplist = identify_opfamily_groups(oprlist, proclist);
+   opclassgroup = NULL;
+   foreach(lc, grouplist)
+   {
+       OpFamilyOpFuncGroup *thisgroup = (OpFamilyOpFuncGroup *) lfirst(lc);
+
+       /* Remember the group exactly matching the test opclass */
+       if (thisgroup->lefttype == opcintype &&
+           thisgroup->righttype == opcintype)
+           opclassgroup = thisgroup;
 
-       /* Count operators that are specifically for the named opclass */
-       if (oprform->amoplefttype == opcintype &&
-           oprform->amoprighttype == opcintype)
-           numclassops++;
+       /*
+        * Complain if there seems to be an incomplete set of operators for
+        * this datatype pair (implying that we have a hash function but no
+        * operator).
+        */
+       if (thisgroup->operatorset != (1 << HTEqualStrategyNumber))
+       {
+           ereport(INFO,
+                   (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                    errmsg("hash opfamily %s is missing operator(s) for types %s and %s",
+                           opfamilyname,
+                           format_type_be(thisgroup->lefttype),
+                           format_type_be(thisgroup->righttype))));
+           result = false;
+       }
    }
 
-   /* Check that the named opclass is complete */
-   if (numclassops != HTMaxStrategyNumber)
-       ereport(ERROR,
+   /* Check that the originally-named opclass is supported */
+   /* (if group is there, we already checked it adequately above) */
+   if (!opclassgroup)
+   {
+       ereport(INFO,
                (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                errmsg("hash opclass %u is missing operator(s)",
-                       opclassoid)));
-   if ((classfuncbits & (1 << HASHPROC)) == 0)
-       ereport(ERROR,
+                errmsg("hash opclass %s is missing operator(s)",
+                       opclassname)));
+       result = false;
+   }
+
+   /*
+    * Complain if the opfamily doesn't have entries for all possible
+    * combinations of its supported datatypes.  While missing cross-type
+    * operators are not fatal, it seems reasonable to insist that all
+    * built-in hash opfamilies be complete.
+    */
+   if (list_length(grouplist) !=
+       list_length(hashabletypes) * list_length(hashabletypes))
+   {
+       ereport(INFO,
                (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-              errmsg("hash opclass %u is missing required support function",
-                     opclassoid)));
+                errmsg("hash opfamily %s is missing cross-type operator(s)",
+                       opfamilyname)));
+       result = false;
+   }
 
    ReleaseCatCacheList(proclist);
    ReleaseCatCacheList(oprlist);
+   ReleaseSysCache(familytup);
+   ReleaseSysCache(classtup);
+
+   return result;
+}
+
+
+/*
+ * We need a custom version of check_amproc_signature because of assorted
+ * hacks in the core hash opclass definitions.
+ */
+static bool
+check_hash_func_signature(Oid funcid, Oid restype, Oid argtype)
+{
+   bool        result = true;
+   HeapTuple   tp;
+   Form_pg_proc procform;
+
+   tp = SearchSysCache1(PROCOID, ObjectIdGetDatum(funcid));
+   if (!HeapTupleIsValid(tp))
+       elog(ERROR, "cache lookup failed for function %u", funcid);
+   procform = (Form_pg_proc) GETSTRUCT(tp);
+
+   if (procform->prorettype != restype || procform->proretset ||
+       procform->pronargs != 1)
+       result = false;
+
+   if (!IsBinaryCoercible(argtype, procform->proargtypes.values[0]))
+   {
+       /*
+        * Some of the built-in hash opclasses cheat by using hash functions
+        * that are different from but physically compatible with the opclass
+        * datatype.  In some of these cases, even a "binary coercible" check
+        * fails because there's no relevant cast.  For the moment, fix it by
+        * having a whitelist of allowed cases.  Test the specific function
+        * identity, not just its input type, because hashvarlena() takes
+        * INTERNAL and allowing any such function seems too scary.
+        */
+       if (funcid == F_HASHINT4 &&
+           (argtype == DATEOID ||
+            argtype == ABSTIMEOID || argtype == RELTIMEOID ||
+            argtype == XIDOID || argtype == CIDOID))
+            /* okay, allowed use of hashint4() */ ;
+       else if (funcid == F_TIMESTAMP_HASH &&
+                argtype == TIMESTAMPTZOID)
+            /* okay, allowed use of timestamp_hash() */ ;
+       else if (funcid == F_HASHCHAR &&
+                argtype == BOOLOID)
+            /* okay, allowed use of hashchar() */ ;
+       else if (funcid == F_HASHVARLENA &&
+                argtype == BYTEAOID)
+            /* okay, allowed use of hashvarlena() */ ;
+       else
+           result = false;
+   }
 
-   return true;
+   ReleaseSysCache(tp);
+   return result;
 }
index b82e5d727f8597123c11460450b0b56beded783f..cb38be8f557cd0552b355ef23b2a6029c0a02aea 100644 (file)
@@ -12,6 +12,6 @@ subdir = src/backend/access/index
 top_builddir = ../../../..
 include $(top_builddir)/src/Makefile.global
 
-OBJS = amapi.o genam.o indexam.o
+OBJS = amapi.o amvalidate.o genam.o indexam.o
 
 include $(top_srcdir)/src/backend/common.mk
diff --git a/src/backend/access/index/amvalidate.c b/src/backend/access/index/amvalidate.c
new file mode 100644 (file)
index 0000000..1a3c5f1
--- /dev/null
@@ -0,0 +1,246 @@
+/*-------------------------------------------------------------------------
+ *
+ * amvalidate.c
+ *   Support routines for index access methods' amvalidate functions.
+ *
+ * Copyright (c) 2016, PostgreSQL Global Development Group
+ *
+ *
+ * IDENTIFICATION
+ *   src/backend/access/index/amvalidate.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "postgres.h"
+
+#include "access/amvalidate.h"
+#include "access/htup_details.h"
+#include "catalog/pg_am.h"
+#include "catalog/pg_amop.h"
+#include "catalog/pg_amproc.h"
+#include "catalog/pg_opclass.h"
+#include "catalog/pg_operator.h"
+#include "catalog/pg_proc.h"
+#include "parser/parse_coerce.h"
+#include "utils/syscache.h"
+
+
+/*
+ * identify_opfamily_groups() returns a List of OpFamilyOpFuncGroup structs,
+ * one for each combination of lefttype/righttype present in the family's
+ * operator and support function lists.  If amopstrategy K is present for
+ * this datatype combination, we set bit 1 << K in operatorset, and similarly
+ * for the support functions.  With uint64 fields we can handle operator and
+ * function numbers up to 63, which is plenty for the foreseeable future.
+ *
+ * The given CatCLists are expected to represent a single opfamily fetched
+ * from the AMOPSTRATEGY and AMPROCNUM caches, so that they will be in
+ * order by those caches' second and third cache keys, namely the datatypes.
+ */
+List *
+identify_opfamily_groups(CatCList *oprlist, CatCList *proclist)
+{
+   List       *result = NIL;
+   OpFamilyOpFuncGroup *thisgroup;
+   Form_pg_amop oprform;
+   Form_pg_amproc procform;
+   int         io,
+               ip;
+
+   /* We need the lists to be ordered; should be true in normal operation */
+   if (!oprlist->ordered || !proclist->ordered)
+       elog(ERROR, "cannot validate operator family without ordered data");
+
+   /*
+    * Advance through the lists concurrently.  Thanks to the ordering, we
+    * should see all operators and functions of a given datatype pair
+    * consecutively.
+    */
+   thisgroup = NULL;
+   io = ip = 0;
+   if (io < oprlist->n_members)
+   {
+       oprform = (Form_pg_amop) GETSTRUCT(&oprlist->members[io]->tuple);
+       io++;
+   }
+   else
+       oprform = NULL;
+   if (ip < proclist->n_members)
+   {
+       procform = (Form_pg_amproc) GETSTRUCT(&proclist->members[ip]->tuple);
+       ip++;
+   }
+   else
+       procform = NULL;
+
+   while (oprform || procform)
+   {
+       if (oprform && thisgroup &&
+           oprform->amoplefttype == thisgroup->lefttype &&
+           oprform->amoprighttype == thisgroup->righttype)
+       {
+           /* Operator belongs to current group; include it and advance */
+
+           /* Ignore strategy numbers outside supported range */
+           if (oprform->amopstrategy > 0 && oprform->amopstrategy < 64)
+               thisgroup->operatorset |= ((uint64) 1) << oprform->amopstrategy;
+
+           if (io < oprlist->n_members)
+           {
+               oprform = (Form_pg_amop) GETSTRUCT(&oprlist->members[io]->tuple);
+               io++;
+           }
+           else
+               oprform = NULL;
+           continue;
+       }
+
+       if (procform && thisgroup &&
+           procform->amproclefttype == thisgroup->lefttype &&
+           procform->amprocrighttype == thisgroup->righttype)
+       {
+           /* Procedure belongs to current group; include it and advance */
+
+           /* Ignore function numbers outside supported range */
+           if (procform->amprocnum > 0 && procform->amprocnum < 64)
+               thisgroup->functionset |= ((uint64) 1) << procform->amprocnum;
+
+           if (ip < proclist->n_members)
+           {
+               procform = (Form_pg_amproc) GETSTRUCT(&proclist->members[ip]->tuple);
+               ip++;
+           }
+           else
+               procform = NULL;
+           continue;
+       }
+
+       /* Time for a new group */
+       thisgroup = (OpFamilyOpFuncGroup *) palloc(sizeof(OpFamilyOpFuncGroup));
+       if (oprform &&
+           (!procform ||
+            (oprform->amoplefttype < procform->amproclefttype ||
+             (oprform->amoplefttype == procform->amproclefttype &&
+              oprform->amoprighttype < procform->amprocrighttype))))
+       {
+           thisgroup->lefttype = oprform->amoplefttype;
+           thisgroup->righttype = oprform->amoprighttype;
+       }
+       else
+       {
+           thisgroup->lefttype = procform->amproclefttype;
+           thisgroup->righttype = procform->amprocrighttype;
+       }
+       thisgroup->operatorset = thisgroup->functionset = 0;
+       result = lappend(result, thisgroup);
+   }
+
+   return result;
+}
+
+/*
+ * Validate the signature (argument and result types) of an opclass support
+ * function.  Return TRUE if OK, FALSE if not.
+ *
+ * The "..." represents maxargs argument-type OIDs.  If "exact" is TRUE, they
+ * must match the function arg types exactly, else only binary-coercibly.
+ * In any case the function result type must match restype exactly.
+ */
+bool
+check_amproc_signature(Oid funcid, Oid restype, bool exact,
+                      int minargs, int maxargs,...)
+{
+   bool        result = true;
+   HeapTuple   tp;
+   Form_pg_proc procform;
+   va_list     ap;
+   int         i;
+
+   tp = SearchSysCache1(PROCOID, ObjectIdGetDatum(funcid));
+   if (!HeapTupleIsValid(tp))
+       elog(ERROR, "cache lookup failed for function %u", funcid);
+   procform = (Form_pg_proc) GETSTRUCT(tp);
+
+   if (procform->prorettype != restype || procform->proretset ||
+       procform->pronargs < minargs || procform->pronargs > maxargs)
+       result = false;
+
+   va_start(ap, maxargs);
+   for (i = 0; i < maxargs; i++)
+   {
+       Oid         argtype = va_arg(ap, Oid);
+
+       if (i >= procform->pronargs)
+           continue;
+       if (exact ? (argtype != procform->proargtypes.values[i]) :
+           !IsBinaryCoercible(argtype, procform->proargtypes.values[i]))
+           result = false;
+   }
+   va_end(ap);
+
+   ReleaseSysCache(tp);
+   return result;
+}
+
+/*
+ * Validate the signature (argument and result types) of an opclass operator.
+ * Return TRUE if OK, FALSE if not.
+ *
+ * Currently, we can hard-wire this as accepting only binary operators.  Also,
+ * we can insist on exact type matches, since the given lefttype/righttype
+ * come from pg_amop and should always match the operator exactly.
+ */
+bool
+check_amop_signature(Oid opno, Oid restype, Oid lefttype, Oid righttype)
+{
+   bool        result = true;
+   HeapTuple   tp;
+   Form_pg_operator opform;
+
+   tp = SearchSysCache1(OPEROID, ObjectIdGetDatum(opno));
+   if (!HeapTupleIsValid(tp))  /* shouldn't happen */
+       elog(ERROR, "cache lookup failed for operator %u", opno);
+   opform = (Form_pg_operator) GETSTRUCT(tp);
+
+   if (opform->oprresult != restype || opform->oprkind != 'b' ||
+       opform->oprleft != lefttype || opform->oprright != righttype)
+       result = false;
+
+   ReleaseSysCache(tp);
+   return result;
+}
+
+/*
+ * Is the datatype a legitimate input type for the btree opfamily?
+ */
+bool
+opfamily_can_sort_type(Oid opfamilyoid, Oid datatypeoid)
+{
+   bool        result = false;
+   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.
+    */
+   opclist = SearchSysCacheList1(CLAAMNAMENSP, ObjectIdGetDatum(BTREE_AM_OID));
+
+   for (i = 0; i < opclist->n_members; i++)
+   {
+       HeapTuple   classtup = &opclist->members[i]->tuple;
+       Form_pg_opclass classform = (Form_pg_opclass) GETSTRUCT(classtup);
+
+       if (classform->opcfamily == opfamilyoid &&
+           classform->opcintype == datatypeoid)
+       {
+           result = true;
+           break;
+       }
+   }
+
+   ReleaseCatCacheList(opclist);
+
+   return result;
+}
index b814b54a36b2ec2b458fb17ec0397d72f71468c7..e05fe4875f85b8ecc03011adc3f6fdc7aa084a67 100644 (file)
  */
 #include "postgres.h"
 
+#include "access/amvalidate.h"
 #include "access/htup_details.h"
 #include "access/nbtree.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/catcache.h"
 #include "utils/syscache.h"
 
 
 bool
 btvalidate(Oid opclassoid)
 {
+   bool        result = true;
    HeapTuple   classtup;
    Form_pg_opclass classform;
    Oid         opfamilyoid;
    Oid         opcintype;
-   int         numclassops;
-   int32       classfuncbits;
+   char       *opclassname;
+   HeapTuple   familytup;
+   Form_pg_opfamily familyform;
+   char       *opfamilyname;
    CatCList   *proclist,
               *oprlist;
-   Oid         lastlefttype,
-               lastrighttype;
-   int         numOps;
-   int         i,
-               j;
+   List       *grouplist;
+   OpFamilyOpFuncGroup *opclassgroup;
+   List       *familytypes;
+   int         i;
+   ListCell   *lc;
 
    /* Fetch opclass information */
    classtup = SearchSysCache1(CLAOID, ObjectIdGetDatum(opclassoid));
@@ -56,45 +61,63 @@ btvalidate(Oid opclassoid)
 
    opfamilyoid = classform->opcfamily;
    opcintype = classform->opcintype;
+   opclassname = NameStr(classform->opcname);
 
-   ReleaseSysCache(classtup);
+   /* Fetch opfamily information */
+   familytup = SearchSysCache1(OPFAMILYOID, ObjectIdGetDatum(opfamilyoid));
+   if (!HeapTupleIsValid(familytup))
+       elog(ERROR, "cache lookup failed for operator family %u", opfamilyoid);
+   familyform = (Form_pg_opfamily) GETSTRUCT(familytup);
+
+   opfamilyname = NameStr(familyform->opfname);
 
    /* Fetch all operators and support functions of the opfamily */
    oprlist = SearchSysCacheList1(AMOPSTRATEGY, ObjectIdGetDatum(opfamilyoid));
    proclist = SearchSysCacheList1(AMPROCNUM, ObjectIdGetDatum(opfamilyoid));
 
-   /* We rely on the oprlist to be ordered */
-   if (!oprlist->ordered)
-       elog(ERROR, "cannot validate btree opclass without ordered data");
-
-   /* We'll track the ops and functions belonging to the named opclass */
-   numclassops = 0;
-   classfuncbits = 0;
-
-   /* Check support functions */
+   /* Check individual support functions */
    for (i = 0; i < proclist->n_members; i++)
    {
        HeapTuple   proctup = &proclist->members[i]->tuple;
        Form_pg_amproc procform = (Form_pg_amproc) GETSTRUCT(proctup);
+       bool        ok;
+
+       /* Check procedure numbers and function signatures */
+       switch (procform->amprocnum)
+       {
+           case BTORDER_PROC:
+               ok = check_amproc_signature(procform->amproc, INT4OID, true,
+                                           2, 2, procform->amproclefttype,
+                                           procform->amprocrighttype);
+               break;
+           case BTSORTSUPPORT_PROC:
+               ok = check_amproc_signature(procform->amproc, VOIDOID, true,
+                                           1, 1, INTERNALOID);
+               break;
+           default:
+               ereport(INFO,
+                       (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                        errmsg("btree opfamily %s contains function %s with invalid support number %d",
+                               opfamilyname,
+                               format_procedure(procform->amproc),
+                               procform->amprocnum)));
+               result = false;
+               continue;       /* don't want additional message */
+       }
 
-       /* Check that only allowed procedure numbers exist */
-       if (procform->amprocnum != BTORDER_PROC &&
-           procform->amprocnum != BTSORTSUPPORT_PROC)
-           ereport(ERROR,
+       if (!ok)
+       {
+           ereport(INFO,
                    (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                    errmsg("btree opfamily %u contains invalid support number %d for procedure %u",
-                           opfamilyoid,
-                           procform->amprocnum, procform->amproc)));
-
-       /* Remember functions that are specifically for the named opclass */
-       if (procform->amproclefttype == opcintype &&
-           procform->amprocrighttype == opcintype)
-           classfuncbits |= (1 << procform->amprocnum);
+                    errmsg("btree opfamily %s contains function %s with wrong signature for support number %d",
+                           opfamilyname,
+                           format_procedure(procform->amproc),
+                           procform->amprocnum)));
+           result = false;
+       }
    }
 
-   /* Check operators */
-   lastlefttype = lastrighttype = InvalidOid;
-   numOps = 0;
+   /* Check individual operators */
    for (i = 0; i < oprlist->n_members; i++)
    {
        HeapTuple   oprtup = &oprlist->members[i]->tuple;
@@ -103,102 +126,127 @@ btvalidate(Oid opclassoid)
        /* Check that only allowed strategy numbers exist */
        if (oprform->amopstrategy < 1 ||
            oprform->amopstrategy > BTMaxStrategyNumber)
-           ereport(ERROR,
+       {
+           ereport(INFO,
                    (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                    errmsg("btree opfamily %u contains invalid strategy number %d for operator %u",
-                           opfamilyoid,
-                           oprform->amopstrategy, oprform->amopopr)));
+                    errmsg("btree opfamily %s contains operator %s with invalid strategy number %d",
+                           opfamilyname,
+                           format_operator(oprform->amopopr),
+                           oprform->amopstrategy)));
+           result = false;
+       }
 
-       /*
-        * Check that we have all strategies for each supported datatype
-        * combination.  This is easy since the list will be sorted in
-        * datatype order and there can't be duplicate strategy numbers.
-        */
-       if (oprform->amoplefttype == lastlefttype &&
-           oprform->amoprighttype == lastrighttype)
-           numOps++;
-       else
+       /* btree doesn't support ORDER BY operators */
+       if (oprform->amoppurpose != AMOP_SEARCH ||
+           OidIsValid(oprform->amopsortfamily))
        {
-           /* reached a group boundary, so check ... */
-           if (numOps > 0 && numOps != BTMaxStrategyNumber)
-               ereport(ERROR,
-                       (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                        errmsg("btree opfamily %u has a partial set of operators for datatypes %s and %s",
-                               opfamilyoid,
-                               format_type_be(lastlefttype),
-                               format_type_be(lastrighttype))));
-           /* ... and reset for new group */
-           lastlefttype = oprform->amoplefttype;
-           lastrighttype = oprform->amoprighttype;
-           numOps = 1;
+           ereport(INFO,
+                   (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                    errmsg("btree opfamily %s contains invalid ORDER BY specification for operator %s",
+                           opfamilyname,
+                           format_operator(oprform->amopopr))));
+           result = false;
+       }
+
+       /* Check operator signature --- same for all btree strategies */
+       if (!check_amop_signature(oprform->amopopr, BOOLOID,
+                                 oprform->amoplefttype,
+                                 oprform->amoprighttype))
+       {
+           ereport(INFO,
+                   (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                    errmsg("btree opfamily %s contains operator %s with wrong signature",
+                           opfamilyname,
+                           format_operator(oprform->amopopr))));
+           result = false;
        }
+   }
+
+   /* Now check for inconsistent groups of operators/functions */
+   grouplist = identify_opfamily_groups(oprlist, proclist);
+   opclassgroup = NULL;
+   familytypes = NIL;
+   foreach(lc, grouplist)
+   {
+       OpFamilyOpFuncGroup *thisgroup = (OpFamilyOpFuncGroup *) lfirst(lc);
+
+       /* Remember the group exactly matching the test opclass */
+       if (thisgroup->lefttype == opcintype &&
+           thisgroup->righttype == opcintype)
+           opclassgroup = thisgroup;
+
+       /*
+        * Identify all distinct data types handled in this opfamily.  This
+        * implementation is O(N^2), but there aren't likely to be enough
+        * types in the family for it to matter.
+        */
+       familytypes = list_append_unique_oid(familytypes, thisgroup->lefttype);
+       familytypes = list_append_unique_oid(familytypes, thisgroup->righttype);
 
        /*
-        * There should be a relevant support function for each operator, but
-        * we only need to check this once per pair of datatypes.
+        * Complain if there seems to be an incomplete set of either operators
+        * or support functions for this datatype pair.  The only thing that
+        * is considered optional is the sortsupport function.
         */
-       if (numOps == 1)
+       if (thisgroup->operatorset !=
+           ((1 << BTLessStrategyNumber) |
+            (1 << BTLessEqualStrategyNumber) |
+            (1 << BTEqualStrategyNumber) |
+            (1 << BTGreaterEqualStrategyNumber) |
+            (1 << BTGreaterStrategyNumber)))
        {
-           bool        found = false;
-
-           for (j = 0; j < proclist->n_members; j++)
-           {
-               HeapTuple   proctup = &proclist->members[j]->tuple;
-               Form_pg_amproc procform = (Form_pg_amproc) GETSTRUCT(proctup);
-
-               if (procform->amprocnum == BTORDER_PROC &&
-                   procform->amproclefttype == oprform->amoplefttype &&
-                   procform->amprocrighttype == oprform->amoprighttype)
-               {
-                   found = true;
-                   break;
-               }
-           }
-
-           if (!found)
-               ereport(ERROR,
-                       (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                        errmsg("btree opfamily %u lacks support function for operator %u",
-                               opfamilyoid, oprform->amopopr)));
+           ereport(INFO,
+                   (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                    errmsg("btree opfamily %s is missing operator(s) for types %s and %s",
+                           opfamilyname,
+                           format_type_be(thisgroup->lefttype),
+                           format_type_be(thisgroup->righttype))));
+           result = false;
        }
-
-       /* btree doesn't support ORDER BY operators */
-       if (oprform->amoppurpose != AMOP_SEARCH ||
-           OidIsValid(oprform->amopsortfamily))
-           ereport(ERROR,
+       if ((thisgroup->functionset & (1 << BTORDER_PROC)) == 0)
+       {
+           ereport(INFO,
                    (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                    errmsg("btree opfamily %u contains invalid ORDER BY specification for operator %u",
-                           opfamilyoid, oprform->amopopr)));
-
-       /* Count operators that are specifically for the named opclass */
-       if (oprform->amoplefttype == opcintype &&
-           oprform->amoprighttype == opcintype)
-           numclassops++;
+                    errmsg("btree opfamily %s is missing support function for types %s and %s",
+                           opfamilyname,
+                           format_type_be(thisgroup->lefttype),
+                           format_type_be(thisgroup->righttype))));
+           result = false;
+       }
    }
 
-   /* don't forget to check the last batch of operators for completeness */
-   if (numOps > 0 && numOps != BTMaxStrategyNumber)
-       ereport(ERROR,
-               (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                errmsg("btree opfamily %u has a partial set of operators for datatypes %s and %s",
-                       opfamilyoid,
-                       format_type_be(lastlefttype),
-                       format_type_be(lastrighttype))));
-
-   /* Check that the named opclass is complete */
-   if (numclassops != BTMaxStrategyNumber)
-       ereport(ERROR,
+   /* Check that the originally-named opclass is supported */
+   /* (if group is there, we already checked it adequately above) */
+   if (!opclassgroup)
+   {
+       ereport(INFO,
                (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                errmsg("btree opclass %u is missing operator(s)",
-                       opclassoid)));
-   if ((classfuncbits & (1 << BTORDER_PROC)) == 0)
-       ereport(ERROR,
+                errmsg("btree opclass %s is missing operator(s)",
+                       opclassname)));
+       result = false;
+   }
+
+   /*
+    * Complain if the opfamily doesn't have entries for all possible
+    * combinations of its supported datatypes.  While missing cross-type
+    * operators are not fatal, they do limit the planner's ability to derive
+    * additional qual clauses from equivalence classes, so it seems
+    * reasonable to insist that all built-in btree opfamilies be complete.
+    */
+   if (list_length(grouplist) !=
+       list_length(familytypes) * list_length(familytypes))
+   {
+       ereport(INFO,
                (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-             errmsg("btree opclass %u is missing required support function",
-                    opclassoid)));
+                errmsg("btree opfamily %s is missing cross-type operator(s)",
+                       opfamilyname)));
+       result = false;
+   }
 
    ReleaseCatCacheList(proclist);
    ReleaseCatCacheList(oprlist);
+   ReleaseSysCache(familytup);
+   ReleaseSysCache(classtup);
 
-   return true;
+   return result;
 }
index c2d2d466d94cebbd755b8e759d630703c3efce3e..ba7c82845312e1c548b939f4c4d64eb715995db0 100644 (file)
  */
 #include "postgres.h"
 
+#include "access/amvalidate.h"
 #include "access/htup_details.h"
 #include "access/spgist_private.h"
 #include "catalog/pg_amop.h"
 #include "catalog/pg_amproc.h"
 #include "catalog/pg_opclass.h"
-#include "utils/catcache.h"
+#include "catalog/pg_opfamily.h"
+#include "catalog/pg_type.h"
+#include "utils/builtins.h"
 #include "utils/syscache.h"
 
 
 /*
  * Validator for an SP-GiST opclass.
+ *
+ * Some of the checks done here cover the whole opfamily, and therefore are
+ * redundant when checking each opclass in a family.  But they don't run long
+ * enough to be much of a problem, so we accept the duplication rather than
+ * complicate the amvalidate API.
  */
 bool
 spgvalidate(Oid opclassoid)
 {
+   bool        result = true;
    HeapTuple   classtup;
    Form_pg_opclass classform;
    Oid         opfamilyoid;
    Oid         opcintype;
-   int         numclassops;
-   int32       classfuncbits;
+   char       *opclassname;
+   HeapTuple   familytup;
+   Form_pg_opfamily familyform;
+   char       *opfamilyname;
    CatCList   *proclist,
               *oprlist;
+   List       *grouplist;
+   OpFamilyOpFuncGroup *opclassgroup;
    int         i;
+   ListCell   *lc;
 
    /* Fetch opclass information */
    classtup = SearchSysCache1(CLAOID, ObjectIdGetDatum(opclassoid));
@@ -46,84 +60,185 @@ spgvalidate(Oid opclassoid)
 
    opfamilyoid = classform->opcfamily;
    opcintype = classform->opcintype;
+   opclassname = NameStr(classform->opcname);
 
-   ReleaseSysCache(classtup);
+   /* Fetch opfamily information */
+   familytup = SearchSysCache1(OPFAMILYOID, ObjectIdGetDatum(opfamilyoid));
+   if (!HeapTupleIsValid(familytup))
+       elog(ERROR, "cache lookup failed for operator family %u", opfamilyoid);
+   familyform = (Form_pg_opfamily) GETSTRUCT(familytup);
+
+   opfamilyname = NameStr(familyform->opfname);
 
    /* Fetch all operators and support functions of the opfamily */
    oprlist = SearchSysCacheList1(AMOPSTRATEGY, ObjectIdGetDatum(opfamilyoid));
    proclist = SearchSysCacheList1(AMPROCNUM, ObjectIdGetDatum(opfamilyoid));
 
-   /* We'll track the ops and functions belonging to the named opclass */
-   numclassops = 0;
-   classfuncbits = 0;
-
-   /* Check support functions */
+   /* Check individual support functions */
    for (i = 0; i < proclist->n_members; i++)
    {
        HeapTuple   proctup = &proclist->members[i]->tuple;
        Form_pg_amproc procform = (Form_pg_amproc) GETSTRUCT(proctup);
-
-       /* Check that only allowed procedure numbers exist */
-       if (procform->amprocnum < 1 ||
-           procform->amprocnum > SPGISTNProc)
-           ereport(ERROR,
+       bool        ok;
+
+       /*
+        * All SP-GiST support functions should be registered with matching
+        * left/right types
+        */
+       if (procform->amproclefttype != procform->amprocrighttype)
+       {
+           ereport(INFO,
+                   (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                    errmsg("spgist opfamily %s contains support procedure %s with cross-type registration",
+                           opfamilyname,
+                           format_procedure(procform->amproc))));
+           result = false;
+       }
+
+       /* Check procedure numbers and function signatures */
+       switch (procform->amprocnum)
+       {
+           case SPGIST_CONFIG_PROC:
+           case SPGIST_CHOOSE_PROC:
+           case SPGIST_PICKSPLIT_PROC:
+           case SPGIST_INNER_CONSISTENT_PROC:
+               ok = check_amproc_signature(procform->amproc, VOIDOID, true,
+                                           2, 2, INTERNALOID, INTERNALOID);
+               break;
+           case SPGIST_LEAF_CONSISTENT_PROC:
+               ok = check_amproc_signature(procform->amproc, BOOLOID, true,
+                                           2, 2, INTERNALOID, INTERNALOID);
+               break;
+           default:
+               ereport(INFO,
+                       (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                        errmsg("spgist opfamily %s contains function %s with invalid support number %d",
+                               opfamilyname,
+                               format_procedure(procform->amproc),
+                               procform->amprocnum)));
+               result = false;
+               continue;       /* don't want additional message */
+       }
+
+       if (!ok)
+       {
+           ereport(INFO,
                    (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                    errmsg("spgist opfamily %u contains invalid support number %d for procedure %u",
-                           opfamilyoid,
-                           procform->amprocnum, procform->amproc)));
-
-       /* Remember functions that are specifically for the named opclass */
-       if (procform->amproclefttype == opcintype &&
-           procform->amprocrighttype == opcintype)
-           classfuncbits |= (1 << procform->amprocnum);
+                    errmsg("spgist opfamily %s contains function %s with wrong signature for support number %d",
+                           opfamilyname,
+                           format_procedure(procform->amproc),
+                           procform->amprocnum)));
+           result = false;
+       }
    }
 
-   /* Check operators */
+   /* Check individual operators */
    for (i = 0; i < oprlist->n_members; i++)
    {
        HeapTuple   oprtup = &oprlist->members[i]->tuple;
        Form_pg_amop oprform = (Form_pg_amop) GETSTRUCT(oprtup);
 
        /* TODO: Check that only allowed strategy numbers exist */
-       if (oprform->amopstrategy < 1)
-           ereport(ERROR,
+       if (oprform->amopstrategy < 1 || oprform->amopstrategy > 63)
+       {
+           ereport(INFO,
                    (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                    errmsg("spgist opfamily %u contains invalid strategy number %d for operator %u",
-                           opfamilyoid,
-                           oprform->amopstrategy, oprform->amopopr)));
+                    errmsg("spgist opfamily %s contains operator %s with invalid strategy number %d",
+                           opfamilyname,
+                           format_operator(oprform->amopopr),
+                           oprform->amopstrategy)));
+           result = false;
+       }
 
        /* spgist doesn't support ORDER BY operators */
        if (oprform->amoppurpose != AMOP_SEARCH ||
            OidIsValid(oprform->amopsortfamily))
-           ereport(ERROR,
+       {
+           ereport(INFO,
+                   (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                    errmsg("spgist opfamily %s contains invalid ORDER BY specification for operator %s",
+                           opfamilyname,
+                           format_operator(oprform->amopopr))));
+           result = false;
+       }
+
+       /* Check operator signature --- same for all spgist strategies */
+       if (!check_amop_signature(oprform->amopopr, BOOLOID,
+                                 oprform->amoplefttype,
+                                 oprform->amoprighttype))
+       {
+           ereport(INFO,
                    (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                    errmsg("spgist opfamily %u contains invalid ORDER BY specification for operator %u",
-                           opfamilyoid, oprform->amopopr)));
+                    errmsg("spgist opfamily %s contains operator %s with wrong signature",
+                           opfamilyname,
+                           format_operator(oprform->amopopr))));
+           result = false;
+       }
+   }
 
-       /* Count operators that are specifically for the named opclass */
-       if (oprform->amoplefttype == opcintype &&
-           oprform->amoprighttype == opcintype)
-           numclassops++;
+   /* Now check for inconsistent groups of operators/functions */
+   grouplist = identify_opfamily_groups(oprlist, proclist);
+   opclassgroup = NULL;
+   foreach(lc, grouplist)
+   {
+       OpFamilyOpFuncGroup *thisgroup = (OpFamilyOpFuncGroup *) lfirst(lc);
+
+       /* Remember the group exactly matching the test opclass */
+       if (thisgroup->lefttype == opcintype &&
+           thisgroup->righttype == opcintype)
+           opclassgroup = thisgroup;
+
+       /*
+        * Complain if there are any datatype pairs with functions but no
+        * operators.  This is about the best we can do for now to detect
+        * missing operators.
+        */
+       if (thisgroup->operatorset == 0)
+       {
+           ereport(INFO,
+                   (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                    errmsg("spgist opfamily %s is missing operator(s) for types %s and %s",
+                           opfamilyname,
+                           format_type_be(thisgroup->lefttype),
+                           format_type_be(thisgroup->righttype))));
+           result = false;
+       }
+
+       /*
+        * Complain if we're missing functions for any datatype, remembering
+        * that SP-GiST doesn't use cross-type support functions.
+        */
+       if (thisgroup->lefttype != thisgroup->righttype)
+           continue;
+
+       for (i = 1; i <= SPGISTNProc; i++)
+       {
+           if ((thisgroup->functionset & (1 << i)) != 0)
+               continue;       /* got it */
+           ereport(INFO,
+                   (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                    errmsg("spgist opfamily %s is missing support function %d for type %s",
+                           opfamilyname, i,
+                           format_type_be(thisgroup->lefttype))));
+           result = false;
+       }
    }
 
-   /* Check that the named opclass is complete */
-   if (numclassops == 0)
-       ereport(ERROR,
-               (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-                errmsg("spgist opclass %u is missing operator(s)",
-                       opclassoid)));
-   for (i = 1; i <= SPGISTNProc; i++)
+   /* Check that the originally-named opclass is supported */
+   /* (if group is there, we already checked it adequately above) */
+   if (!opclassgroup)
    {
-       if ((classfuncbits & (1 << i)) != 0)
-           continue;           /* got it */
-       ereport(ERROR,
+       ereport(INFO,
                (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
-         errmsg("spgist opclass %u is missing required support function %d",
-                opclassoid, i)));
+                errmsg("spgist opclass %s is missing operator(s)",
+                       opclassname)));
+       result = false;
    }
 
    ReleaseCatCacheList(proclist);
    ReleaseCatCacheList(oprlist);
+   ReleaseSysCache(familytup);
+   ReleaseSysCache(classtup);
 
-   return true;
+   return result;
 }
index a180d2b507a1b9855c2075eed48ba9174c5bd36a..cb26d79afb05eead608391b3e69f4ce7e42ae34e 100644 (file)
@@ -1102,6 +1102,29 @@ get_opname(Oid opno)
        return NULL;
 }
 
+/*
+ * get_op_rettype
+ *     Given operator oid, return the operator's result type.
+ */
+Oid
+get_op_rettype(Oid opno)
+{
+   HeapTuple   tp;
+
+   tp = SearchSysCache1(OPEROID, ObjectIdGetDatum(opno));
+   if (HeapTupleIsValid(tp))
+   {
+       Form_pg_operator optup = (Form_pg_operator) GETSTRUCT(tp);
+       Oid         result;
+
+       result = optup->oprresult;
+       ReleaseSysCache(tp);
+       return result;
+   }
+   else
+       return InvalidOid;
+}
+
 /*
  * op_input_types
  *
diff --git a/src/include/access/amvalidate.h b/src/include/access/amvalidate.h
new file mode 100644 (file)
index 0000000..7f4289a
--- /dev/null
@@ -0,0 +1,36 @@
+/*-------------------------------------------------------------------------
+ *
+ * amvalidate.h
+ *   Support routines for index access methods' amvalidate functions.
+ *
+ * Copyright (c) 2016, PostgreSQL Global Development Group
+ *
+ * src/include/access/amvalidate.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef AMVALIDATE_H
+#define AMVALIDATE_H
+
+#include "utils/catcache.h"
+
+
+/* Struct returned (in a list) by identify_opfamily_groups() */
+typedef struct OpFamilyOpFuncGroup
+{
+   Oid         lefttype;       /* amoplefttype/amproclefttype */
+   Oid         righttype;      /* amoprighttype/amprocrighttype */
+   uint64      operatorset;    /* bitmask of operators with these types */
+   uint64      functionset;    /* bitmask of support funcs with these types */
+} OpFamilyOpFuncGroup;
+
+
+/* Functions in access/index/amvalidate.c */
+extern List *identify_opfamily_groups(CatCList *oprlist, CatCList *proclist);
+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 bool opfamily_can_sort_type(Oid opfamilyoid, Oid datatypeoid);
+
+#endif   /* AMVALIDATE_H */
index 9efdea22fd183ae9461a0b291c282f71466df6e5..dcb89807e9d2232947a407979a153a9eca7799eb 100644 (file)
@@ -75,6 +75,7 @@ extern Oid    get_opclass_family(Oid opclass);
 extern Oid get_opclass_input_type(Oid opclass);
 extern RegProcedure get_opcode(Oid opno);
 extern char *get_opname(Oid opno);
+extern Oid get_op_rettype(Oid opno);
 extern void op_input_types(Oid opno, Oid *lefttype, Oid *righttype);
 extern bool op_mergejoinable(Oid opno, Oid inputtype);
 extern bool op_hashjoinable(Oid opno, Oid inputtype);
index 45f13f3d0676cb9b2415d7d5b48d017f2496a77c..7c09fa3d590f22e80865615aa377b1f1ca4f33ed 100644 (file)
@@ -1567,6 +1567,7 @@ WHERE p1.oid != p2.oid AND
 (0 rows)
 
 -- Ask access methods to validate opclasses
+-- (this replaces a lot of SQL-level checks that used to be done in this file)
 SELECT oid, opcname FROM pg_opclass WHERE NOT amvalidate(oid);
  oid | opcname 
 -----+---------
@@ -1610,15 +1611,6 @@ WHERE NOT ((p1.amoppurpose = 's' AND p1.amopsortfamily = 0) OR
 ------------+--------------
 (0 rows)
 
--- amoplefttype/amoprighttype must match the operator
-SELECT p1.oid, p2.oid
-FROM pg_amop AS p1, pg_operator AS p2
-WHERE p1.amopopr = p2.oid AND NOT
-    (p1.amoplefttype = p2.oprleft AND p1.amoprighttype = p2.oprright);
- oid | oid 
------+-----
-(0 rows)
-
 -- amopmethod must match owning opfamily's opfmethod
 SELECT p1.oid, p2.oid
 FROM pg_amop AS p1, pg_opfamily AS p2
@@ -1627,44 +1619,6 @@ WHERE p1.amopfamily = p2.oid AND p1.amopmethod != p2.opfmethod;
 -----+-----
 (0 rows)
 
--- amopsortfamily, if present, must reference a btree family
-SELECT p1.amopfamily, p1.amopstrategy
-FROM pg_amop AS p1
-WHERE p1.amopsortfamily <> 0 AND NOT EXISTS
-    (SELECT 1 from pg_opfamily op WHERE op.oid = p1.amopsortfamily
-     AND op.opfmethod = (SELECT oid FROM pg_am WHERE amname = 'btree'));
- amopfamily | amopstrategy 
-------------+--------------
-(0 rows)
-
--- Check that amopopr points at a reasonable-looking operator, ie a binary
--- operator.  If it's a search operator it had better yield boolean,
--- otherwise an input type of its sort opfamily.
-SELECT p1.amopfamily, p1.amopopr, p2.oid, p2.oprname
-FROM pg_amop AS p1, pg_operator AS p2
-WHERE p1.amopopr = p2.oid AND
-    p2.oprkind != 'b';
- amopfamily | amopopr | oid | oprname 
-------------+---------+-----+---------
-(0 rows)
-
-SELECT p1.amopfamily, p1.amopopr, p2.oid, p2.oprname
-FROM pg_amop AS p1, pg_operator AS p2
-WHERE p1.amopopr = p2.oid AND p1.amoppurpose = 's' AND
-    p2.oprresult != 'bool'::regtype;
- amopfamily | amopopr | oid | oprname 
-------------+---------+-----+---------
-(0 rows)
-
-SELECT p1.amopfamily, p1.amopopr, p2.oid, p2.oprname
-FROM pg_amop AS p1, pg_operator AS p2
-WHERE p1.amopopr = p2.oid AND p1.amoppurpose = 'o' AND NOT EXISTS
-    (SELECT 1 FROM pg_opclass op
-     WHERE opcfamily = p1.amopsortfamily AND opcintype = p2.oprresult);
- amopfamily | amopopr | oid | oprname 
-------------+---------+-----+---------
-(0 rows)
-
 -- Make a list of all the distinct operator names being used in particular
 -- strategy slots.  This is a bit hokey, since the list might need to change
 -- in future releases, but it's an effective way of spotting mistakes such as
@@ -1843,72 +1797,6 @@ WHERE p1.amopopr = p2.oid AND p2.oprcode = p3.oid AND
 ------------+---------+---------+--------
 (0 rows)
 
--- Multiple-datatype btree opfamilies should provide closed sets of equality
--- operators; that is if you provide int2 = int4 and int4 = int8 then you
--- should also provide int2 = int8 (and commutators of all these).  This is
--- important because the planner tries to deduce additional qual clauses from
--- transitivity of mergejoinable operators.  If there are clauses
--- int2var = int4var and int4var = int8var, the planner will want to deduce
--- int2var = int8var ... so there should be a way to represent that.  While
--- a missing cross-type operator is now only an efficiency loss rather than
--- an error condition, it still seems reasonable to insist that all built-in
--- opfamilies be complete.
--- check commutative closure
-SELECT p1.amoplefttype, p1.amoprighttype
-FROM pg_amop AS p1
-WHERE p1.amopmethod = (SELECT oid FROM pg_am WHERE amname = 'btree') AND
-    p1.amopstrategy = 3 AND
-    p1.amoplefttype != p1.amoprighttype AND
-    NOT EXISTS(SELECT 1 FROM pg_amop p2 WHERE
-                 p2.amopfamily = p1.amopfamily AND
-                 p2.amoplefttype = p1.amoprighttype AND
-                 p2.amoprighttype = p1.amoplefttype AND
-                 p2.amopstrategy = 3);
- amoplefttype | amoprighttype 
---------------+---------------
-(0 rows)
-
--- check transitive closure
-SELECT p1.amoplefttype, p1.amoprighttype, p2.amoprighttype
-FROM pg_amop AS p1, pg_amop AS p2
-WHERE p1.amopfamily = p2.amopfamily AND
-    p1.amoprighttype = p2.amoplefttype AND
-    p1.amopmethod = (SELECT oid FROM pg_am WHERE amname = 'btree') AND
-    p2.amopmethod = (SELECT oid FROM pg_am WHERE amname = 'btree') AND
-    p1.amopstrategy = 3 AND p2.amopstrategy = 3 AND
-    p1.amoplefttype != p1.amoprighttype AND
-    p2.amoplefttype != p2.amoprighttype AND
-    NOT EXISTS(SELECT 1 FROM pg_amop p3 WHERE
-                 p3.amopfamily = p1.amopfamily AND
-                 p3.amoplefttype = p1.amoplefttype AND
-                 p3.amoprighttype = p2.amoprighttype AND
-                 p3.amopstrategy = 3);
- amoplefttype | amoprighttype | amoprighttype 
---------------+---------------+---------------
-(0 rows)
-
--- We also expect that built-in multiple-datatype hash opfamilies provide
--- complete sets of cross-type operators.  Again, this isn't required, but
--- it is reasonable to expect it for built-in opfamilies.
--- if same family has x=x and y=y, it should have x=y
-SELECT p1.amoplefttype, p2.amoplefttype
-FROM pg_amop AS p1, pg_amop AS p2
-WHERE p1.amopfamily = p2.amopfamily AND
-    p1.amoplefttype = p1.amoprighttype AND
-    p2.amoplefttype = p2.amoprighttype AND
-    p1.amopmethod = (SELECT oid FROM pg_am WHERE amname = 'hash') AND
-    p2.amopmethod = (SELECT oid FROM pg_am WHERE amname = 'hash') AND
-    p1.amopstrategy = 1 AND p2.amopstrategy = 1 AND
-    p1.amoplefttype != p2.amoplefttype AND
-    NOT EXISTS(SELECT 1 FROM pg_amop p3 WHERE
-                 p3.amopfamily = p1.amopfamily AND
-                 p3.amoplefttype = p1.amoplefttype AND
-                 p3.amoprighttype = p2.amoplefttype AND
-                 p3.amopstrategy = 1);
- amoplefttype | amoplefttype 
---------------+--------------
-(0 rows)
-
 -- **************** pg_amproc ****************
 -- Look for illegal values in pg_amproc fields
 SELECT p1.amprocfamily, p1.amprocnum
@@ -1919,96 +1807,6 @@ WHERE p1.amprocfamily = 0 OR p1.amproclefttype = 0 OR p1.amprocrighttype = 0
 --------------+-----------
 (0 rows)
 
--- Unfortunately, we can't check the amproc link very well because the
--- signature of the function may be different for different support routines
--- or different base data types.
--- We can check that all the referenced instances of the same support
--- routine number take the same number of parameters, but that's about it
--- for a general check...
-SELECT p1.amprocfamily, p1.amprocnum,
-   p2.oid, p2.proname,
-   p3.opfname,
-   p4.amprocfamily, p4.amprocnum,
-   p5.oid, p5.proname,
-   p6.opfname
-FROM pg_amproc AS p1, pg_proc AS p2, pg_opfamily AS p3,
-     pg_amproc AS p4, pg_proc AS p5, pg_opfamily AS p6
-WHERE p1.amprocfamily = p3.oid AND p4.amprocfamily = p6.oid AND
-    p3.opfmethod = p6.opfmethod AND p1.amprocnum = p4.amprocnum AND
-    p1.amproc = p2.oid AND p4.amproc = p5.oid AND
-    (p2.proretset OR p5.proretset OR p2.pronargs != p5.pronargs);
- amprocfamily | amprocnum | oid | proname | opfname | amprocfamily | amprocnum | oid | proname | opfname 
---------------+-----------+-----+---------+---------+--------------+-----------+-----+---------+---------
-(0 rows)
-
--- For btree, though, we can do better since we know the support routines
--- must be of the form cmp(lefttype, righttype) returns int4
--- or sortsupport(internal) returns void.
-SELECT p1.amprocfamily, p1.amprocnum,
-   p2.oid, p2.proname,
-   p3.opfname
-FROM pg_amproc AS p1, pg_proc AS p2, pg_opfamily AS p3
-WHERE p3.opfmethod = (SELECT oid FROM pg_am WHERE amname = 'btree')
-    AND p1.amprocfamily = p3.oid AND p1.amproc = p2.oid AND
-    (CASE WHEN amprocnum = 1
-          THEN prorettype != 'int4'::regtype OR proretset OR pronargs != 2
-               OR proargtypes[0] != amproclefttype
-               OR proargtypes[1] != amprocrighttype
-          WHEN amprocnum = 2
-          THEN prorettype != 'void'::regtype OR proretset OR pronargs != 1
-               OR proargtypes[0] != 'internal'::regtype
-          ELSE true END);
- amprocfamily | amprocnum | oid | proname | opfname 
---------------+-----------+-----+---------+---------
-(0 rows)
-
--- For hash we can also do a little better: the support routines must be
--- of the form hash(lefttype) returns int4.  There are several cases where
--- we cheat and use a hash function that is physically compatible with the
--- datatype even though there's no cast, so this check does find a small
--- number of entries.
-SELECT p1.amprocfamily, p1.amprocnum, p2.proname, p3.opfname
-FROM pg_amproc AS p1, pg_proc AS p2, pg_opfamily AS p3
-WHERE p3.opfmethod = (SELECT oid FROM pg_am WHERE amname = 'hash')
-    AND p1.amprocfamily = p3.oid AND p1.amproc = p2.oid AND
-    (amprocnum != 1
-     OR proretset
-     OR prorettype != 'int4'::regtype
-     OR pronargs != 1
-     OR NOT physically_coercible(amproclefttype, proargtypes[0])
-     OR amproclefttype != amprocrighttype)
-ORDER BY 1;
- amprocfamily | amprocnum |    proname     |     opfname     
---------------+-----------+----------------+-----------------
-          435 |         1 | hashint4       | date_ops
-         1999 |         1 | timestamp_hash | timestamptz_ops
-         2222 |         1 | hashchar       | bool_ops
-         2223 |         1 | hashvarlena    | bytea_ops
-         2225 |         1 | hashint4       | xid_ops
-         2226 |         1 | hashint4       | cid_ops
-(6 rows)
-
--- We can also check SP-GiST carefully, since the support routine signatures
--- are independent of the datatype being indexed.
-SELECT p1.amprocfamily, p1.amprocnum,
-   p2.oid, p2.proname,
-   p3.opfname
-FROM pg_amproc AS p1, pg_proc AS p2, pg_opfamily AS p3
-WHERE p3.opfmethod = (SELECT oid FROM pg_am WHERE amname = 'spgist')
-    AND p1.amprocfamily = p3.oid AND p1.amproc = p2.oid AND
-    (CASE WHEN amprocnum = 1 OR amprocnum = 2 OR amprocnum = 3 OR amprocnum = 4
-          THEN prorettype != 'void'::regtype OR proretset OR pronargs != 2
-               OR proargtypes[0] != 'internal'::regtype
-               OR proargtypes[1] != 'internal'::regtype
-          WHEN amprocnum = 5
-          THEN prorettype != 'bool'::regtype OR proretset OR pronargs != 2
-               OR proargtypes[0] != 'internal'::regtype
-               OR proargtypes[1] != 'internal'::regtype
-          ELSE true END);
- amprocfamily | amprocnum | oid | proname | opfname 
---------------+-----------+-----+---------+---------
-(0 rows)
-
 -- Support routines that are primary members of opfamilies must be immutable
 -- (else it suggests that the index ordering isn't fixed).  But cross-type
 -- members need only be stable, since they are just shorthands
index c42c8a35616b1ebc744920c85d8b0ba132b06432..6c9784a387281928ff3023e4794edd9213000e96 100644 (file)
@@ -1037,6 +1037,7 @@ WHERE p1.oid != p2.oid AND
     p1.opcdefault AND p2.opcdefault;
 
 -- Ask access methods to validate opclasses
+-- (this replaces a lot of SQL-level checks that used to be done in this file)
 
 SELECT oid, opcname FROM pg_opclass WHERE NOT amvalidate(oid);
 
@@ -1073,47 +1074,12 @@ FROM pg_amop as p1
 WHERE NOT ((p1.amoppurpose = 's' AND p1.amopsortfamily = 0) OR
            (p1.amoppurpose = 'o' AND p1.amopsortfamily <> 0));
 
--- amoplefttype/amoprighttype must match the operator
-
-SELECT p1.oid, p2.oid
-FROM pg_amop AS p1, pg_operator AS p2
-WHERE p1.amopopr = p2.oid AND NOT
-    (p1.amoplefttype = p2.oprleft AND p1.amoprighttype = p2.oprright);
-
 -- amopmethod must match owning opfamily's opfmethod
 
 SELECT p1.oid, p2.oid
 FROM pg_amop AS p1, pg_opfamily AS p2
 WHERE p1.amopfamily = p2.oid AND p1.amopmethod != p2.opfmethod;
 
--- amopsortfamily, if present, must reference a btree family
-
-SELECT p1.amopfamily, p1.amopstrategy
-FROM pg_amop AS p1
-WHERE p1.amopsortfamily <> 0 AND NOT EXISTS
-    (SELECT 1 from pg_opfamily op WHERE op.oid = p1.amopsortfamily
-     AND op.opfmethod = (SELECT oid FROM pg_am WHERE amname = 'btree'));
-
--- Check that amopopr points at a reasonable-looking operator, ie a binary
--- operator.  If it's a search operator it had better yield boolean,
--- otherwise an input type of its sort opfamily.
-
-SELECT p1.amopfamily, p1.amopopr, p2.oid, p2.oprname
-FROM pg_amop AS p1, pg_operator AS p2
-WHERE p1.amopopr = p2.oid AND
-    p2.oprkind != 'b';
-
-SELECT p1.amopfamily, p1.amopopr, p2.oid, p2.oprname
-FROM pg_amop AS p1, pg_operator AS p2
-WHERE p1.amopopr = p2.oid AND p1.amoppurpose = 's' AND
-    p2.oprresult != 'bool'::regtype;
-
-SELECT p1.amopfamily, p1.amopopr, p2.oid, p2.oprname
-FROM pg_amop AS p1, pg_operator AS p2
-WHERE p1.amopopr = p2.oid AND p1.amoppurpose = 'o' AND NOT EXISTS
-    (SELECT 1 FROM pg_opclass op
-     WHERE opcfamily = p1.amopsortfamily AND opcintype = p2.oprresult);
-
 -- Make a list of all the distinct operator names being used in particular
 -- strategy slots.  This is a bit hokey, since the list might need to change
 -- in future releases, but it's an effective way of spotting mistakes such as
@@ -1171,65 +1137,6 @@ WHERE p1.amopopr = p2.oid AND p2.oprcode = p3.oid AND
     p1.amoplefttype != p1.amoprighttype AND
     p3.provolatile = 'v';
 
--- Multiple-datatype btree opfamilies should provide closed sets of equality
--- operators; that is if you provide int2 = int4 and int4 = int8 then you
--- should also provide int2 = int8 (and commutators of all these).  This is
--- important because the planner tries to deduce additional qual clauses from
--- transitivity of mergejoinable operators.  If there are clauses
--- int2var = int4var and int4var = int8var, the planner will want to deduce
--- int2var = int8var ... so there should be a way to represent that.  While
--- a missing cross-type operator is now only an efficiency loss rather than
--- an error condition, it still seems reasonable to insist that all built-in
--- opfamilies be complete.
-
--- check commutative closure
-SELECT p1.amoplefttype, p1.amoprighttype
-FROM pg_amop AS p1
-WHERE p1.amopmethod = (SELECT oid FROM pg_am WHERE amname = 'btree') AND
-    p1.amopstrategy = 3 AND
-    p1.amoplefttype != p1.amoprighttype AND
-    NOT EXISTS(SELECT 1 FROM pg_amop p2 WHERE
-                 p2.amopfamily = p1.amopfamily AND
-                 p2.amoplefttype = p1.amoprighttype AND
-                 p2.amoprighttype = p1.amoplefttype AND
-                 p2.amopstrategy = 3);
-
--- check transitive closure
-SELECT p1.amoplefttype, p1.amoprighttype, p2.amoprighttype
-FROM pg_amop AS p1, pg_amop AS p2
-WHERE p1.amopfamily = p2.amopfamily AND
-    p1.amoprighttype = p2.amoplefttype AND
-    p1.amopmethod = (SELECT oid FROM pg_am WHERE amname = 'btree') AND
-    p2.amopmethod = (SELECT oid FROM pg_am WHERE amname = 'btree') AND
-    p1.amopstrategy = 3 AND p2.amopstrategy = 3 AND
-    p1.amoplefttype != p1.amoprighttype AND
-    p2.amoplefttype != p2.amoprighttype AND
-    NOT EXISTS(SELECT 1 FROM pg_amop p3 WHERE
-                 p3.amopfamily = p1.amopfamily AND
-                 p3.amoplefttype = p1.amoplefttype AND
-                 p3.amoprighttype = p2.amoprighttype AND
-                 p3.amopstrategy = 3);
-
--- We also expect that built-in multiple-datatype hash opfamilies provide
--- complete sets of cross-type operators.  Again, this isn't required, but
--- it is reasonable to expect it for built-in opfamilies.
-
--- if same family has x=x and y=y, it should have x=y
-SELECT p1.amoplefttype, p2.amoplefttype
-FROM pg_amop AS p1, pg_amop AS p2
-WHERE p1.amopfamily = p2.amopfamily AND
-    p1.amoplefttype = p1.amoprighttype AND
-    p2.amoplefttype = p2.amoprighttype AND
-    p1.amopmethod = (SELECT oid FROM pg_am WHERE amname = 'hash') AND
-    p2.amopmethod = (SELECT oid FROM pg_am WHERE amname = 'hash') AND
-    p1.amopstrategy = 1 AND p2.amopstrategy = 1 AND
-    p1.amoplefttype != p2.amoplefttype AND
-    NOT EXISTS(SELECT 1 FROM pg_amop p3 WHERE
-                 p3.amopfamily = p1.amopfamily AND
-                 p3.amoplefttype = p1.amoplefttype AND
-                 p3.amoprighttype = p2.amoplefttype AND
-                 p3.amopstrategy = 1);
-
 
 -- **************** pg_amproc ****************
 
@@ -1240,82 +1147,6 @@ FROM pg_amproc as p1
 WHERE p1.amprocfamily = 0 OR p1.amproclefttype = 0 OR p1.amprocrighttype = 0
     OR p1.amprocnum < 1 OR p1.amproc = 0;
 
--- Unfortunately, we can't check the amproc link very well because the
--- signature of the function may be different for different support routines
--- or different base data types.
--- We can check that all the referenced instances of the same support
--- routine number take the same number of parameters, but that's about it
--- for a general check...
-
-SELECT p1.amprocfamily, p1.amprocnum,
-   p2.oid, p2.proname,
-   p3.opfname,
-   p4.amprocfamily, p4.amprocnum,
-   p5.oid, p5.proname,
-   p6.opfname
-FROM pg_amproc AS p1, pg_proc AS p2, pg_opfamily AS p3,
-     pg_amproc AS p4, pg_proc AS p5, pg_opfamily AS p6
-WHERE p1.amprocfamily = p3.oid AND p4.amprocfamily = p6.oid AND
-    p3.opfmethod = p6.opfmethod AND p1.amprocnum = p4.amprocnum AND
-    p1.amproc = p2.oid AND p4.amproc = p5.oid AND
-    (p2.proretset OR p5.proretset OR p2.pronargs != p5.pronargs);
-
--- For btree, though, we can do better since we know the support routines
--- must be of the form cmp(lefttype, righttype) returns int4
--- or sortsupport(internal) returns void.
-
-SELECT p1.amprocfamily, p1.amprocnum,
-   p2.oid, p2.proname,
-   p3.opfname
-FROM pg_amproc AS p1, pg_proc AS p2, pg_opfamily AS p3
-WHERE p3.opfmethod = (SELECT oid FROM pg_am WHERE amname = 'btree')
-    AND p1.amprocfamily = p3.oid AND p1.amproc = p2.oid AND
-    (CASE WHEN amprocnum = 1
-          THEN prorettype != 'int4'::regtype OR proretset OR pronargs != 2
-               OR proargtypes[0] != amproclefttype
-               OR proargtypes[1] != amprocrighttype
-          WHEN amprocnum = 2
-          THEN prorettype != 'void'::regtype OR proretset OR pronargs != 1
-               OR proargtypes[0] != 'internal'::regtype
-          ELSE true END);
-
--- For hash we can also do a little better: the support routines must be
--- of the form hash(lefttype) returns int4.  There are several cases where
--- we cheat and use a hash function that is physically compatible with the
--- datatype even though there's no cast, so this check does find a small
--- number of entries.
-
-SELECT p1.amprocfamily, p1.amprocnum, p2.proname, p3.opfname
-FROM pg_amproc AS p1, pg_proc AS p2, pg_opfamily AS p3
-WHERE p3.opfmethod = (SELECT oid FROM pg_am WHERE amname = 'hash')
-    AND p1.amprocfamily = p3.oid AND p1.amproc = p2.oid AND
-    (amprocnum != 1
-     OR proretset
-     OR prorettype != 'int4'::regtype
-     OR pronargs != 1
-     OR NOT physically_coercible(amproclefttype, proargtypes[0])
-     OR amproclefttype != amprocrighttype)
-ORDER BY 1;
-
--- We can also check SP-GiST carefully, since the support routine signatures
--- are independent of the datatype being indexed.
-
-SELECT p1.amprocfamily, p1.amprocnum,
-   p2.oid, p2.proname,
-   p3.opfname
-FROM pg_amproc AS p1, pg_proc AS p2, pg_opfamily AS p3
-WHERE p3.opfmethod = (SELECT oid FROM pg_am WHERE amname = 'spgist')
-    AND p1.amprocfamily = p3.oid AND p1.amproc = p2.oid AND
-    (CASE WHEN amprocnum = 1 OR amprocnum = 2 OR amprocnum = 3 OR amprocnum = 4
-          THEN prorettype != 'void'::regtype OR proretset OR pronargs != 2
-               OR proargtypes[0] != 'internal'::regtype
-               OR proargtypes[1] != 'internal'::regtype
-          WHEN amprocnum = 5
-          THEN prorettype != 'bool'::regtype OR proretset OR pronargs != 2
-               OR proargtypes[0] != 'internal'::regtype
-               OR proargtypes[1] != 'internal'::regtype
-          ELSE true END);
-
 -- Support routines that are primary members of opfamilies must be immutable
 -- (else it suggests that the index ordering isn't fixed).  But cross-type
 -- members need only be stable, since they are just shorthands