diff --git a/contrib/bloom/blutils.c b/contrib/bloom/blutils.c index d3bf8665df1..26b9927c3aa 100644 --- a/contrib/bloom/blutils.c +++ b/contrib/bloom/blutils.c @@ -139,6 +139,7 @@ blhandler(PG_FUNCTION_ARGS) amroutine->amproperty = NULL; amroutine->ambuildphasename = NULL; amroutine->amvalidate = blvalidate; + amroutine->amadjustmembers = NULL; amroutine->ambeginscan = blbeginscan; amroutine->amrescan = blrescan; amroutine->amgettuple = NULL; diff --git a/doc/src/sgml/indexam.sgml b/doc/src/sgml/indexam.sgml index af87f172a7c..1aea4db707d 100644 --- a/doc/src/sgml/indexam.sgml +++ b/doc/src/sgml/indexam.sgml @@ -143,6 +143,7 @@ typedef struct IndexAmRoutine amproperty_function amproperty; /* can be NULL */ ambuildphasename_function ambuildphasename; /* can be NULL */ amvalidate_function amvalidate; + amadjustmembers_function amadjustmembers; /* can be NULL */ ambeginscan_function ambeginscan; amrescan_function amrescan; amgettuple_function amgettuple; /* can be NULL */ @@ -502,7 +503,48 @@ amvalidate (Oid opclassoid); the access method can reasonably do that. For example, this might include testing that all required support functions are provided. The amvalidate function must return false if the opclass is - invalid. Problems should be reported with ereport messages. + invalid. Problems should be reported with ereport + messages, typically at INFO level. + + + + +void +amadjustmembers (Oid opfamilyoid, + Oid opclassoid, + List *operators, + List *functions); + + Validate proposed new operator and function members of an operator family, + so far as the access method can reasonably do that, and set their + dependency types if the default is not satisfactory. This is called + during CREATE OPERATOR CLASS and during + ALTER OPERATOR FAMILY ADD; in the latter + case opclassoid is InvalidOid. + The List arguments are lists + of OpFamilyMember structs, as defined + in amapi.h. + + Tests done by this function will typically be a subset of those + performed by amvalidate, + since amadjustmembers cannot assume that it is + seeing a complete set of members. For example, it would be reasonable + to check the signature of a support function, but not to check whether + all required support functions are provided. Any problems can be + reported by throwing an error. + + The dependency-related fields of + the OpFamilyMember structs are initialized by + the core code to create hard dependencies on the opclass if this + is CREATE OPERATOR CLASS, or soft dependencies on the + opfamily if this is ALTER OPERATOR FAMILY ADD. + amadjustmembers can adjust these fields if some other + behavior is more appropriate. For example, GIN, GiST, and SP-GiST + always set operator members to have soft dependencies on the opfamily, + since the connection between an operator and an opclass is relatively + weak in these index types; so it is reasonable to allow operator members + to be added and removed freely. Optional support functions are typically + also given soft dependencies, so that they can be removed if necessary. diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c index 7db3ae5ee0c..1f72562c603 100644 --- a/src/backend/access/brin/brin.c +++ b/src/backend/access/brin/brin.c @@ -120,6 +120,7 @@ brinhandler(PG_FUNCTION_ARGS) amroutine->amproperty = NULL; amroutine->ambuildphasename = NULL; amroutine->amvalidate = brinvalidate; + amroutine->amadjustmembers = NULL; amroutine->ambeginscan = brinbeginscan; amroutine->amrescan = brinrescan; amroutine->amgettuple = NULL; diff --git a/src/backend/access/gin/ginutil.c b/src/backend/access/gin/ginutil.c index a400f1fedbc..ef9b56fd363 100644 --- a/src/backend/access/gin/ginutil.c +++ b/src/backend/access/gin/ginutil.c @@ -71,6 +71,7 @@ ginhandler(PG_FUNCTION_ARGS) amroutine->amproperty = NULL; amroutine->ambuildphasename = NULL; amroutine->amvalidate = ginvalidate; + amroutine->amadjustmembers = ginadjustmembers; amroutine->ambeginscan = ginbeginscan; amroutine->amrescan = ginrescan; amroutine->amgettuple = NULL; diff --git a/src/backend/access/gin/ginvalidate.c b/src/backend/access/gin/ginvalidate.c index 1e3046f4eb7..60ce1ae1066 100644 --- a/src/backend/access/gin/ginvalidate.c +++ b/src/backend/access/gin/ginvalidate.c @@ -271,3 +271,68 @@ ginvalidate(Oid opclassoid) return result; } + +/* + * Prechecking function for adding operators/functions to a GIN opfamily. + */ +void +ginadjustmembers(Oid opfamilyoid, + Oid opclassoid, + List *operators, + List *functions) +{ + ListCell *lc; + + /* + * Operator members of a GIN opfamily should never have hard dependencies, + * since their connection to the opfamily depends only on what the support + * functions think, and that can be altered. For consistency, we make all + * soft dependencies point to the opfamily, though a soft dependency on + * the opclass would work as well in the CREATE OPERATOR CLASS case. + */ + foreach(lc, operators) + { + OpFamilyMember *op = (OpFamilyMember *) lfirst(lc); + + op->ref_is_hard = false; + op->ref_is_family = true; + op->refobjid = opfamilyoid; + } + + /* + * Required support functions should have hard dependencies. Preferably + * those are just dependencies on the opclass, but if we're in ALTER + * OPERATOR FAMILY, we leave the dependency pointing at the whole + * opfamily. (Given that GIN opclasses generally don't share opfamilies, + * it seems unlikely to be worth working harder.) + */ + foreach(lc, functions) + { + OpFamilyMember *op = (OpFamilyMember *) lfirst(lc); + + switch (op->number) + { + case GIN_EXTRACTVALUE_PROC: + case GIN_EXTRACTQUERY_PROC: + /* Required support function */ + op->ref_is_hard = true; + break; + case GIN_COMPARE_PROC: + case GIN_CONSISTENT_PROC: + case GIN_COMPARE_PARTIAL_PROC: + case GIN_TRICONSISTENT_PROC: + case GIN_OPTIONS_PROC: + /* Optional, so force it to be a soft family dependency */ + op->ref_is_hard = false; + op->ref_is_family = true; + op->refobjid = opfamilyoid; + break; + default: + ereport(ERROR, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("support function number %d is invalid for access method %s", + op->number, "gin"))); + break; + } + } +} diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c index 79fe6eb8d62..25b42e38f22 100644 --- a/src/backend/access/gist/gist.c +++ b/src/backend/access/gist/gist.c @@ -92,6 +92,7 @@ gisthandler(PG_FUNCTION_ARGS) amroutine->amproperty = gistproperty; amroutine->ambuildphasename = NULL; amroutine->amvalidate = gistvalidate; + amroutine->amadjustmembers = gistadjustmembers; amroutine->ambeginscan = gistbeginscan; amroutine->amrescan = gistrescan; amroutine->amgettuple = gistgettuple; diff --git a/src/backend/access/gist/gistvalidate.c b/src/backend/access/gist/gistvalidate.c index a285736a810..2b9ab693be1 100644 --- a/src/backend/access/gist/gistvalidate.c +++ b/src/backend/access/gist/gistvalidate.c @@ -279,3 +279,72 @@ gistvalidate(Oid opclassoid) return result; } + +/* + * Prechecking function for adding operators/functions to a GiST opfamily. + */ +void +gistadjustmembers(Oid opfamilyoid, + Oid opclassoid, + List *operators, + List *functions) +{ + ListCell *lc; + + /* + * Operator members of a GiST opfamily should never have hard + * dependencies, since their connection to the opfamily depends only on + * what the support functions think, and that can be altered. For + * consistency, we make all soft dependencies point to the opfamily, + * though a soft dependency on the opclass would work as well in the + * CREATE OPERATOR CLASS case. + */ + foreach(lc, operators) + { + OpFamilyMember *op = (OpFamilyMember *) lfirst(lc); + + op->ref_is_hard = false; + op->ref_is_family = true; + op->refobjid = opfamilyoid; + } + + /* + * Required support functions should have hard dependencies. Preferably + * those are just dependencies on the opclass, but if we're in ALTER + * OPERATOR FAMILY, we leave the dependency pointing at the whole + * opfamily. (Given that GiST opclasses generally don't share opfamilies, + * it seems unlikely to be worth working harder.) + */ + foreach(lc, functions) + { + OpFamilyMember *op = (OpFamilyMember *) lfirst(lc); + + switch (op->number) + { + case GIST_CONSISTENT_PROC: + case GIST_UNION_PROC: + case GIST_PENALTY_PROC: + case GIST_PICKSPLIT_PROC: + case GIST_EQUAL_PROC: + /* Required support function */ + op->ref_is_hard = true; + break; + case GIST_COMPRESS_PROC: + case GIST_DECOMPRESS_PROC: + case GIST_DISTANCE_PROC: + case GIST_FETCH_PROC: + case GIST_OPTIONS_PROC: + /* Optional, so force it to be a soft family dependency */ + op->ref_is_hard = false; + op->ref_is_family = true; + op->refobjid = opfamilyoid; + break; + default: + ereport(ERROR, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("support function number %d is invalid for access method %s", + op->number, "gist"))); + break; + } + } +} diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c index 3ec6d528e77..7c9ccf446c8 100644 --- a/src/backend/access/hash/hash.c +++ b/src/backend/access/hash/hash.c @@ -89,6 +89,7 @@ hashhandler(PG_FUNCTION_ARGS) amroutine->amproperty = NULL; amroutine->ambuildphasename = NULL; amroutine->amvalidate = hashvalidate; + amroutine->amadjustmembers = hashadjustmembers; amroutine->ambeginscan = hashbeginscan; amroutine->amrescan = hashrescan; amroutine->amgettuple = hashgettuple; diff --git a/src/backend/access/hash/hashvalidate.c b/src/backend/access/hash/hashvalidate.c index 6f14a9fb455..0fe97e8276b 100644 --- a/src/backend/access/hash/hashvalidate.c +++ b/src/backend/access/hash/hashvalidate.c @@ -16,6 +16,8 @@ #include "access/amvalidate.h" #include "access/hash.h" #include "access/htup_details.h" +#include "access/xact.h" +#include "catalog/pg_am.h" #include "catalog/pg_amop.h" #include "catalog/pg_amproc.h" #include "catalog/pg_opclass.h" @@ -25,6 +27,7 @@ #include "parser/parse_coerce.h" #include "utils/builtins.h" #include "utils/fmgroids.h" +#include "utils/lsyscache.h" #include "utils/regproc.h" #include "utils/syscache.h" @@ -341,3 +344,96 @@ check_hash_func_signature(Oid funcid, int16 amprocnum, Oid argtype) ReleaseSysCache(tp); return result; } + +/* + * Prechecking function for adding operators/functions to a hash opfamily. + */ +void +hashadjustmembers(Oid opfamilyoid, + Oid opclassoid, + List *operators, + List *functions) +{ + Oid opcintype; + ListCell *lc; + + /* + * Hash operators and required support functions are always "loose" + * members of the opfamily if they are cross-type. If they are not + * cross-type, we prefer to tie them to the appropriate opclass ... but if + * the user hasn't created one, we can't do that, and must fall back to + * using the opfamily dependency. (We mustn't force creation of an + * opclass in such a case, as leaving an incomplete opclass laying about + * would be bad. Throwing an error is another undesirable alternative.) + * + * This behavior results in a bit of a dump/reload hazard, in that the + * order of restoring objects could affect what dependencies we end up + * with. pg_dump's existing behavior will preserve the dependency choices + * in most cases, but not if a cross-type operator has been bound tightly + * into an opclass. That's a mistake anyway, so silently "fixing" it + * isn't awful. + * + * Optional support functions are always "loose" family members. + * + * To avoid repeated lookups, we remember the most recently used opclass's + * input type. + */ + if (OidIsValid(opclassoid)) + { + /* During CREATE OPERATOR CLASS, need CCI to see the pg_opclass row */ + CommandCounterIncrement(); + opcintype = get_opclass_input_type(opclassoid); + } + else + opcintype = InvalidOid; + + /* + * We handle operators and support functions almost identically, so rather + * than duplicate this code block, just join the lists. + */ + foreach(lc, list_concat_copy(operators, functions)) + { + OpFamilyMember *op = (OpFamilyMember *) lfirst(lc); + + if (op->is_func && op->number != HASHSTANDARD_PROC) + { + /* Optional support proc, so always a soft family dependency */ + op->ref_is_hard = false; + op->ref_is_family = true; + op->refobjid = opfamilyoid; + } + else if (op->lefttype != op->righttype) + { + /* Cross-type, so always a soft family dependency */ + op->ref_is_hard = false; + op->ref_is_family = true; + op->refobjid = opfamilyoid; + } + else + { + /* Not cross-type; is there a suitable opclass? */ + if (op->lefttype != opcintype) + { + /* Avoid repeating this expensive lookup, even if it fails */ + opcintype = op->lefttype; + opclassoid = opclass_for_family_datatype(HASH_AM_OID, + opfamilyoid, + opcintype); + } + if (OidIsValid(opclassoid)) + { + /* Hard dependency on opclass */ + op->ref_is_hard = true; + op->ref_is_family = false; + op->refobjid = opclassoid; + } + else + { + /* We're stuck, so make a soft dependency on the opfamily */ + op->ref_is_hard = false; + op->ref_is_family = true; + op->refobjid = opfamilyoid; + } + } + } +} diff --git a/src/backend/access/index/amvalidate.c b/src/backend/access/index/amvalidate.c index 24d49750ada..b58c34aa5f2 100644 --- a/src/backend/access/index/amvalidate.c +++ b/src/backend/access/index/amvalidate.c @@ -1,7 +1,8 @@ /*------------------------------------------------------------------------- * * amvalidate.c - * Support routines for index access methods' amvalidate functions. + * Support routines for index access methods' amvalidate and + * amadjustmembers functions. * * Copyright (c) 2016-2020, PostgreSQL Global Development Group * @@ -222,21 +223,28 @@ check_amop_signature(Oid opno, Oid restype, Oid lefttype, Oid righttype) } /* - * Is the datatype a legitimate input type for the btree opfamily? + * Get the OID of the opclass belonging to an opfamily and accepting + * the specified type as input type. Returns InvalidOid if no such opclass. + * + * If there is more than one such opclass, you get a random one of them. + * Since that shouldn't happen, we don't waste cycles checking. + * + * We could look up the AM's OID from the opfamily, but all existing callers + * know that or can get it without an extra lookup, so we make them pass it. */ -bool -opfamily_can_sort_type(Oid opfamilyoid, Oid datatypeoid) +Oid +opclass_for_family_datatype(Oid amoid, Oid opfamilyoid, Oid datatypeoid) { - bool result = false; + Oid result = InvalidOid; CatCList *opclist; int i; /* - * We search through all btree opclasses to see if one matches. This is a - * bit inefficient but there is no better index available. It also saves - * making an explicit check that the opfamily belongs to btree. + * We search through all the AM's opclasses to see if one matches. This + * is a bit inefficient but there is no better index available. It also + * saves making an explicit check that the opfamily belongs to the AM. */ - opclist = SearchSysCacheList1(CLAAMNAMENSP, ObjectIdGetDatum(BTREE_AM_OID)); + opclist = SearchSysCacheList1(CLAAMNAMENSP, ObjectIdGetDatum(amoid)); for (i = 0; i < opclist->n_members; i++) { @@ -246,7 +254,7 @@ opfamily_can_sort_type(Oid opfamilyoid, Oid datatypeoid) if (classform->opcfamily == opfamilyoid && classform->opcintype == datatypeoid) { - result = true; + result = classform->oid; break; } } @@ -255,3 +263,14 @@ opfamily_can_sort_type(Oid opfamilyoid, Oid datatypeoid) return result; } + +/* + * Is the datatype a legitimate input type for the btree opfamily? + */ +bool +opfamily_can_sort_type(Oid opfamilyoid, Oid datatypeoid) +{ + return OidIsValid(opclass_for_family_datatype(BTREE_AM_OID, + opfamilyoid, + datatypeoid)); +} diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c index d65f4357cc8..49a8a9708e3 100644 --- a/src/backend/access/nbtree/nbtree.c +++ b/src/backend/access/nbtree/nbtree.c @@ -141,6 +141,7 @@ bthandler(PG_FUNCTION_ARGS) amroutine->amproperty = btproperty; amroutine->ambuildphasename = btbuildphasename; amroutine->amvalidate = btvalidate; + amroutine->amadjustmembers = btadjustmembers; amroutine->ambeginscan = btbeginscan; amroutine->amrescan = btrescan; amroutine->amgettuple = btgettuple; diff --git a/src/backend/access/nbtree/nbtvalidate.c b/src/backend/access/nbtree/nbtvalidate.c index 02905f79c82..5be728ad07c 100644 --- a/src/backend/access/nbtree/nbtvalidate.c +++ b/src/backend/access/nbtree/nbtvalidate.c @@ -16,12 +16,15 @@ #include "access/amvalidate.h" #include "access/htup_details.h" #include "access/nbtree.h" +#include "access/xact.h" +#include "catalog/pg_am.h" #include "catalog/pg_amop.h" #include "catalog/pg_amproc.h" #include "catalog/pg_opclass.h" #include "catalog/pg_opfamily.h" #include "catalog/pg_type.h" #include "utils/builtins.h" +#include "utils/lsyscache.h" #include "utils/regproc.h" #include "utils/syscache.h" @@ -282,3 +285,96 @@ btvalidate(Oid opclassoid) return result; } + +/* + * Prechecking function for adding operators/functions to a btree opfamily. + */ +void +btadjustmembers(Oid opfamilyoid, + Oid opclassoid, + List *operators, + List *functions) +{ + Oid opcintype; + ListCell *lc; + + /* + * Btree operators and comparison support functions are always "loose" + * members of the opfamily if they are cross-type. If they are not + * cross-type, we prefer to tie them to the appropriate opclass ... but if + * the user hasn't created one, we can't do that, and must fall back to + * using the opfamily dependency. (We mustn't force creation of an + * opclass in such a case, as leaving an incomplete opclass laying about + * would be bad. Throwing an error is another undesirable alternative.) + * + * This behavior results in a bit of a dump/reload hazard, in that the + * order of restoring objects could affect what dependencies we end up + * with. pg_dump's existing behavior will preserve the dependency choices + * in most cases, but not if a cross-type operator has been bound tightly + * into an opclass. That's a mistake anyway, so silently "fixing" it + * isn't awful. + * + * Optional support functions are always "loose" family members. + * + * To avoid repeated lookups, we remember the most recently used opclass's + * input type. + */ + if (OidIsValid(opclassoid)) + { + /* During CREATE OPERATOR CLASS, need CCI to see the pg_opclass row */ + CommandCounterIncrement(); + opcintype = get_opclass_input_type(opclassoid); + } + else + opcintype = InvalidOid; + + /* + * We handle operators and support functions almost identically, so rather + * than duplicate this code block, just join the lists. + */ + foreach(lc, list_concat_copy(operators, functions)) + { + OpFamilyMember *op = (OpFamilyMember *) lfirst(lc); + + if (op->is_func && op->number != BTORDER_PROC) + { + /* Optional support proc, so always a soft family dependency */ + op->ref_is_hard = false; + op->ref_is_family = true; + op->refobjid = opfamilyoid; + } + else if (op->lefttype != op->righttype) + { + /* Cross-type, so always a soft family dependency */ + op->ref_is_hard = false; + op->ref_is_family = true; + op->refobjid = opfamilyoid; + } + else + { + /* Not cross-type; is there a suitable opclass? */ + if (op->lefttype != opcintype) + { + /* Avoid repeating this expensive lookup, even if it fails */ + opcintype = op->lefttype; + opclassoid = opclass_for_family_datatype(BTREE_AM_OID, + opfamilyoid, + opcintype); + } + if (OidIsValid(opclassoid)) + { + /* Hard dependency on opclass */ + op->ref_is_hard = true; + op->ref_is_family = false; + op->refobjid = opclassoid; + } + else + { + /* We're stuck, so make a soft dependency on the opfamily */ + op->ref_is_hard = false; + op->ref_is_family = true; + op->refobjid = opfamilyoid; + } + } + } +} diff --git a/src/backend/access/spgist/spgutils.c b/src/backend/access/spgist/spgutils.c index 0efe05e552b..64d3ba82887 100644 --- a/src/backend/access/spgist/spgutils.c +++ b/src/backend/access/spgist/spgutils.c @@ -74,6 +74,7 @@ spghandler(PG_FUNCTION_ARGS) amroutine->amproperty = spgproperty; amroutine->ambuildphasename = NULL; amroutine->amvalidate = spgvalidate; + amroutine->amadjustmembers = spgadjustmembers; amroutine->ambeginscan = spgbeginscan; amroutine->amrescan = spgrescan; amroutine->amgettuple = spggettuple; diff --git a/src/backend/access/spgist/spgvalidate.c b/src/backend/access/spgist/spgvalidate.c index f0cfd8b42b1..d4f5841e265 100644 --- a/src/backend/access/spgist/spgvalidate.c +++ b/src/backend/access/spgist/spgvalidate.c @@ -303,3 +303,69 @@ spgvalidate(Oid opclassoid) return result; } + +/* + * Prechecking function for adding operators/functions to an SP-GiST opfamily. + */ +void +spgadjustmembers(Oid opfamilyoid, + Oid opclassoid, + List *operators, + List *functions) +{ + ListCell *lc; + + /* + * Operator members of an SP-GiST opfamily should never have hard + * dependencies, since their connection to the opfamily depends only on + * what the support functions think, and that can be altered. For + * consistency, we make all soft dependencies point to the opfamily, + * though a soft dependency on the opclass would work as well in the + * CREATE OPERATOR CLASS case. + */ + foreach(lc, operators) + { + OpFamilyMember *op = (OpFamilyMember *) lfirst(lc); + + op->ref_is_hard = false; + op->ref_is_family = true; + op->refobjid = opfamilyoid; + } + + /* + * Required support functions should have hard dependencies. Preferably + * those are just dependencies on the opclass, but if we're in ALTER + * OPERATOR FAMILY, we leave the dependency pointing at the whole + * opfamily. (Given that SP-GiST opclasses generally don't share + * opfamilies, it seems unlikely to be worth working harder.) + */ + foreach(lc, functions) + { + OpFamilyMember *op = (OpFamilyMember *) lfirst(lc); + + switch (op->number) + { + case SPGIST_CONFIG_PROC: + case SPGIST_CHOOSE_PROC: + case SPGIST_PICKSPLIT_PROC: + case SPGIST_INNER_CONSISTENT_PROC: + case SPGIST_LEAF_CONSISTENT_PROC: + /* Required support function */ + op->ref_is_hard = true; + break; + case SPGIST_COMPRESS_PROC: + case SPGIST_OPTIONS_PROC: + /* Optional, so force it to be a soft family dependency */ + op->ref_is_hard = false; + op->ref_is_family = true; + op->refobjid = opfamilyoid; + break; + default: + ereport(ERROR, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("support function number %d is invalid for access method %s", + op->number, "spgist"))); + break; + } + } +} diff --git a/src/backend/commands/opclasscmds.c b/src/backend/commands/opclasscmds.c index 351866f9f22..28395d5946f 100644 --- a/src/backend/commands/opclasscmds.c +++ b/src/backend/commands/opclasscmds.c @@ -27,7 +27,6 @@ #include "catalog/dependency.h" #include "catalog/indexing.h" #include "catalog/objectaccess.h" -#include "catalog/opfam_internal.h" #include "catalog/pg_am.h" #include "catalog/pg_amop.h" #include "catalog/pg_amproc.h" @@ -62,12 +61,10 @@ static void processTypesSpec(List *args, Oid *lefttype, Oid *righttype); static void assignOperTypes(OpFamilyMember *member, Oid amoid, Oid typeoid); static void assignProcTypes(OpFamilyMember *member, Oid amoid, Oid typeoid, int opclassOptsProcNum); -static void addFamilyMember(List **list, OpFamilyMember *member, bool isProc); -static void storeOperators(List *opfamilyname, Oid amoid, - Oid opfamilyoid, Oid opclassoid, +static void addFamilyMember(List **list, OpFamilyMember *member); +static void storeOperators(List *opfamilyname, Oid amoid, Oid opfamilyoid, List *operators, bool isAdd); -static void storeProcedures(List *opfamilyname, Oid amoid, - Oid opfamilyoid, Oid opclassoid, +static void storeProcedures(List *opfamilyname, Oid amoid, Oid opfamilyoid, List *procedures, bool isAdd); static void dropOperators(List *opfamilyname, Oid amoid, Oid opfamilyoid, List *operators); @@ -518,11 +515,12 @@ DefineOpClass(CreateOpClassStmt *stmt) /* Save the info */ member = (OpFamilyMember *) palloc0(sizeof(OpFamilyMember)); + member->is_func = false; member->object = operOid; member->number = item->number; member->sortfamily = sortfamilyOid; assignOperTypes(member, amoid, typeoid); - addFamilyMember(&operators, member, false); + addFamilyMember(&operators, member); break; case OPCLASS_ITEM_FUNCTION: if (item->number <= 0 || item->number > maxProcNumber) @@ -541,6 +539,7 @@ DefineOpClass(CreateOpClassStmt *stmt) #endif /* Save the info */ member = (OpFamilyMember *) palloc0(sizeof(OpFamilyMember)); + member->is_func = true; member->object = funcOid; member->number = item->number; @@ -550,7 +549,7 @@ DefineOpClass(CreateOpClassStmt *stmt) &member->lefttype, &member->righttype); assignProcTypes(member, amoid, typeoid, optsProcNumber); - addFamilyMember(&procedures, member, true); + addFamilyMember(&procedures, member); break; case OPCLASS_ITEM_STORAGETYPE: if (OidIsValid(storageoid)) @@ -662,14 +661,46 @@ DefineOpClass(CreateOpClassStmt *stmt) heap_freetuple(tup); + /* + * Now that we have the opclass OID, set up default dependency info for + * the pg_amop and pg_amproc entries. Historically, CREATE OPERATOR CLASS + * has created hard dependencies on the opclass, so that's what we use. + */ + foreach(l, operators) + { + OpFamilyMember *op = (OpFamilyMember *) lfirst(l); + + op->ref_is_hard = true; + op->ref_is_family = false; + op->refobjid = opclassoid; + } + foreach(l, procedures) + { + OpFamilyMember *proc = (OpFamilyMember *) lfirst(l); + + proc->ref_is_hard = true; + proc->ref_is_family = false; + proc->refobjid = opclassoid; + } + + /* + * Let the index AM editorialize on the dependency choices. It could also + * do further validation on the operators and functions, if it likes. + */ + if (amroutine->amadjustmembers) + amroutine->amadjustmembers(opfamilyoid, + opclassoid, + operators, + procedures); + /* * Now add tuples to pg_amop and pg_amproc tying in the operators and * functions. Dependencies on them are inserted, too. */ storeOperators(stmt->opfamilyname, amoid, opfamilyoid, - opclassoid, operators, false); + operators, false); storeProcedures(stmt->opfamilyname, amoid, opfamilyoid, - opclassoid, procedures, false); + procedures, false); /* let event triggers know what happened */ EventTriggerCollectCreateOpClass(stmt, opclassoid, operators, procedures); @@ -842,6 +873,7 @@ AlterOpFamilyAdd(AlterOpFamilyStmt *stmt, Oid amoid, Oid opfamilyoid, int maxOpNumber, int maxProcNumber, int optsProcNumber, List *items) { + IndexAmRoutine *amroutine = GetIndexAmRoutineByAmId(amoid, false); List *operators; /* OpFamilyMember list for operators */ List *procedures; /* OpFamilyMember list for support procs */ ListCell *l; @@ -900,11 +932,17 @@ AlterOpFamilyAdd(AlterOpFamilyStmt *stmt, Oid amoid, Oid opfamilyoid, /* Save the info */ member = (OpFamilyMember *) palloc0(sizeof(OpFamilyMember)); + member->is_func = false; member->object = operOid; member->number = item->number; member->sortfamily = sortfamilyOid; + /* We can set up dependency fields immediately */ + /* Historically, ALTER ADD has created soft dependencies */ + member->ref_is_hard = false; + member->ref_is_family = true; + member->refobjid = opfamilyoid; assignOperTypes(member, amoid, InvalidOid); - addFamilyMember(&operators, member, false); + addFamilyMember(&operators, member); break; case OPCLASS_ITEM_FUNCTION: if (item->number <= 0 || item->number > maxProcNumber) @@ -924,8 +962,14 @@ AlterOpFamilyAdd(AlterOpFamilyStmt *stmt, Oid amoid, Oid opfamilyoid, /* Save the info */ member = (OpFamilyMember *) palloc0(sizeof(OpFamilyMember)); + member->is_func = true; member->object = funcOid; member->number = item->number; + /* We can set up dependency fields immediately */ + /* Historically, ALTER ADD has created soft dependencies */ + member->ref_is_hard = false; + member->ref_is_family = true; + member->refobjid = opfamilyoid; /* allow overriding of the function's actual arg types */ if (item->class_args) @@ -933,7 +977,7 @@ AlterOpFamilyAdd(AlterOpFamilyStmt *stmt, Oid amoid, Oid opfamilyoid, &member->lefttype, &member->righttype); assignProcTypes(member, amoid, InvalidOid, optsProcNumber); - addFamilyMember(&procedures, member, true); + addFamilyMember(&procedures, member); break; case OPCLASS_ITEM_STORAGETYPE: ereport(ERROR, @@ -946,14 +990,24 @@ AlterOpFamilyAdd(AlterOpFamilyStmt *stmt, Oid amoid, Oid opfamilyoid, } } + /* + * Let the index AM editorialize on the dependency choices. It could also + * do further validation on the operators and functions, if it likes. + */ + if (amroutine->amadjustmembers) + amroutine->amadjustmembers(opfamilyoid, + InvalidOid, /* no specific opclass */ + operators, + procedures); + /* * Add tuples to pg_amop and pg_amproc tying in the operators and * functions. Dependencies on them are inserted, too. */ storeOperators(stmt->opfamilyname, amoid, opfamilyoid, - InvalidOid, operators, true); + operators, true); storeProcedures(stmt->opfamilyname, amoid, opfamilyoid, - InvalidOid, procedures, true); + procedures, true); /* make information available to event triggers */ EventTriggerCollectAlterOpFam(stmt, opfamilyoid, @@ -996,10 +1050,11 @@ AlterOpFamilyDrop(AlterOpFamilyStmt *stmt, Oid amoid, Oid opfamilyoid, processTypesSpec(item->class_args, &lefttype, &righttype); /* Save the info */ member = (OpFamilyMember *) palloc0(sizeof(OpFamilyMember)); + member->is_func = false; member->number = item->number; member->lefttype = lefttype; member->righttype = righttype; - addFamilyMember(&operators, member, false); + addFamilyMember(&operators, member); break; case OPCLASS_ITEM_FUNCTION: if (item->number <= 0 || item->number > maxProcNumber) @@ -1011,10 +1066,11 @@ AlterOpFamilyDrop(AlterOpFamilyStmt *stmt, Oid amoid, Oid opfamilyoid, processTypesSpec(item->class_args, &lefttype, &righttype); /* Save the info */ member = (OpFamilyMember *) palloc0(sizeof(OpFamilyMember)); + member->is_func = true; member->number = item->number; member->lefttype = lefttype; member->righttype = righttype; - addFamilyMember(&procedures, member, true); + addFamilyMember(&procedures, member); break; case OPCLASS_ITEM_STORAGETYPE: /* grammar prevents this from appearing */ @@ -1324,7 +1380,7 @@ assignProcTypes(OpFamilyMember *member, Oid amoid, Oid typeoid, * duplicated strategy or proc number. */ static void -addFamilyMember(List **list, OpFamilyMember *member, bool isProc) +addFamilyMember(List **list, OpFamilyMember *member) { ListCell *l; @@ -1336,7 +1392,7 @@ addFamilyMember(List **list, OpFamilyMember *member, bool isProc) old->lefttype == member->lefttype && old->righttype == member->righttype) { - if (isProc) + if (member->is_func) ereport(ERROR, (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), errmsg("function number %d for (%s,%s) appears more than once", @@ -1358,13 +1414,10 @@ addFamilyMember(List **list, OpFamilyMember *member, bool isProc) /* * Dump the operators to pg_amop * - * We also make dependency entries in pg_depend for the opfamily entries. - * If opclassoid is valid then make an INTERNAL dependency on that opclass, - * else make an AUTO dependency on the opfamily. + * We also make dependency entries in pg_depend for the pg_amop entries. */ static void -storeOperators(List *opfamilyname, Oid amoid, - Oid opfamilyoid, Oid opclassoid, +storeOperators(List *opfamilyname, Oid amoid, Oid opfamilyoid, List *operators, bool isAdd) { Relation rel; @@ -1434,28 +1487,17 @@ storeOperators(List *opfamilyname, Oid amoid, referenced.objectId = op->object; referenced.objectSubId = 0; - if (OidIsValid(opclassoid)) - { - /* if contained in an opclass, use a NORMAL dep on operator */ - recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL); + /* see comments in amapi.h about dependency strength */ + recordDependencyOn(&myself, &referenced, + op->ref_is_hard ? DEPENDENCY_NORMAL : DEPENDENCY_AUTO); - /* ... and an INTERNAL dep on the opclass */ - referenced.classId = OperatorClassRelationId; - referenced.objectId = opclassoid; - referenced.objectSubId = 0; - recordDependencyOn(&myself, &referenced, DEPENDENCY_INTERNAL); - } - else - { - /* if "loose" in the opfamily, use a AUTO dep on operator */ - recordDependencyOn(&myself, &referenced, DEPENDENCY_AUTO); + referenced.classId = op->ref_is_family ? OperatorFamilyRelationId : + OperatorClassRelationId; + referenced.objectId = op->refobjid; + referenced.objectSubId = 0; - /* ... and an AUTO dep on the opfamily */ - referenced.classId = OperatorFamilyRelationId; - referenced.objectId = opfamilyoid; - referenced.objectSubId = 0; - recordDependencyOn(&myself, &referenced, DEPENDENCY_AUTO); - } + recordDependencyOn(&myself, &referenced, + op->ref_is_hard ? DEPENDENCY_INTERNAL : DEPENDENCY_AUTO); /* A search operator also needs a dep on the referenced opfamily */ if (OidIsValid(op->sortfamily)) @@ -1463,8 +1505,11 @@ storeOperators(List *opfamilyname, Oid amoid, referenced.classId = OperatorFamilyRelationId; referenced.objectId = op->sortfamily; referenced.objectSubId = 0; - recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL); + + recordDependencyOn(&myself, &referenced, + op->ref_is_hard ? DEPENDENCY_NORMAL : DEPENDENCY_AUTO); } + /* Post create hook of this access method operator */ InvokeObjectPostCreateHook(AccessMethodOperatorRelationId, entryoid, 0); @@ -1476,13 +1521,10 @@ storeOperators(List *opfamilyname, Oid amoid, /* * Dump the procedures (support routines) to pg_amproc * - * We also make dependency entries in pg_depend for the opfamily entries. - * If opclassoid is valid then make an INTERNAL dependency on that opclass, - * else make an AUTO dependency on the opfamily. + * We also make dependency entries in pg_depend for the pg_amproc entries. */ static void -storeProcedures(List *opfamilyname, Oid amoid, - Oid opfamilyoid, Oid opclassoid, +storeProcedures(List *opfamilyname, Oid amoid, Oid opfamilyoid, List *procedures, bool isAdd) { Relation rel; @@ -1546,28 +1588,18 @@ storeProcedures(List *opfamilyname, Oid amoid, referenced.objectId = proc->object; referenced.objectSubId = 0; - if (OidIsValid(opclassoid)) - { - /* if contained in an opclass, use a NORMAL dep on procedure */ - recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL); + /* see comments in amapi.h about dependency strength */ + recordDependencyOn(&myself, &referenced, + proc->ref_is_hard ? DEPENDENCY_NORMAL : DEPENDENCY_AUTO); - /* ... and an INTERNAL dep on the opclass */ - referenced.classId = OperatorClassRelationId; - referenced.objectId = opclassoid; - referenced.objectSubId = 0; - recordDependencyOn(&myself, &referenced, DEPENDENCY_INTERNAL); - } - else - { - /* if "loose" in the opfamily, use a AUTO dep on procedure */ - recordDependencyOn(&myself, &referenced, DEPENDENCY_AUTO); + referenced.classId = proc->ref_is_family ? OperatorFamilyRelationId : + OperatorClassRelationId; + referenced.objectId = proc->refobjid; + referenced.objectSubId = 0; + + recordDependencyOn(&myself, &referenced, + proc->ref_is_hard ? DEPENDENCY_INTERNAL : DEPENDENCY_AUTO); - /* ... and an AUTO dep on the opfamily */ - referenced.classId = OperatorFamilyRelationId; - referenced.objectId = opfamilyoid; - referenced.objectSubId = 0; - recordDependencyOn(&myself, &referenced, DEPENDENCY_AUTO); - } /* Post create hook of access method procedure */ InvokeObjectPostCreateHook(AccessMethodProcedureRelationId, entryoid, 0); diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl index e116235769b..ec636620601 100644 --- a/src/bin/pg_dump/t/002_pg_dump.pl +++ b/src/bin/pg_dump/t/002_pg_dump.pl @@ -524,6 +524,8 @@ my %tests = ( FUNCTION 1 (int4, int4) btint4cmp(int4,int4), FUNCTION 2 (int4, int4) btint4sortsupport(internal), FUNCTION 4 (int4, int4) btequalimage(oid);', + # note: it's correct that btint8sortsupport and bigint btequalimage + # are included here: regexp => qr/^ \QALTER OPERATOR FAMILY dump_test.op_family USING btree ADD\E\n\s+ \QOPERATOR 1 <(bigint,integer) ,\E\n\s+ @@ -532,7 +534,9 @@ my %tests = ( \QOPERATOR 4 >=(bigint,integer) ,\E\n\s+ \QOPERATOR 5 >(bigint,integer) ,\E\n\s+ \QFUNCTION 1 (integer, integer) btint4cmp(integer,integer) ,\E\n\s+ + \QFUNCTION 2 (bigint, bigint) btint8sortsupport(internal) ,\E\n\s+ \QFUNCTION 2 (integer, integer) btint4sortsupport(internal) ,\E\n\s+ + \QFUNCTION 4 (bigint, bigint) btequalimage(oid) ,\E\n\s+ \QFUNCTION 4 (integer, integer) btequalimage(oid);\E /xm, like => @@ -1559,6 +1563,8 @@ my %tests = ( FUNCTION 1 btint8cmp(bigint,bigint), FUNCTION 2 btint8sortsupport(internal), FUNCTION 4 btequalimage(oid);', + # note: it's correct that btint8sortsupport and btequalimage + # are NOT included here (they're optional support functions): regexp => qr/^ \QCREATE OPERATOR CLASS dump_test.op_class\E\n\s+ \QFOR TYPE bigint USING btree FAMILY dump_test.op_family AS\E\n\s+ @@ -1567,9 +1573,7 @@ my %tests = ( \QOPERATOR 3 =(bigint,bigint) ,\E\n\s+ \QOPERATOR 4 >=(bigint,bigint) ,\E\n\s+ \QOPERATOR 5 >(bigint,bigint) ,\E\n\s+ - \QFUNCTION 1 (bigint, bigint) btint8cmp(bigint,bigint) ,\E\n\s+ - \QFUNCTION 2 (bigint, bigint) btint8sortsupport(internal) ,\E\n\s+ - \QFUNCTION 4 (bigint, bigint) btequalimage(oid);\E + \QFUNCTION 1 (bigint, bigint) btint8cmp(bigint,bigint);\E /xm, like => { %full_runs, %dump_test_schema_runs, section_pre_data => 1, }, diff --git a/src/include/access/amapi.h b/src/include/access/amapi.h index 4325faa460b..85b4766016f 100644 --- a/src/include/access/amapi.h +++ b/src/include/access/amapi.h @@ -54,6 +54,42 @@ typedef enum IndexAMProperty AMPROP_CAN_INCLUDE } IndexAMProperty; +/* + * We use lists of this struct type to keep track of both operators and + * support functions while building or adding to an opclass or opfamily. + * amadjustmembers functions receive lists of these structs, and are allowed + * to alter their "ref" fields. + * + * The "ref" fields define how the pg_amop or pg_amproc entry should depend + * on the associated objects (that is, which dependency type to use, and + * which opclass or opfamily it should depend on). + * + * If ref_is_hard is true, the entry will have a NORMAL dependency on the + * operator or support func, and an INTERNAL dependency on the opclass or + * opfamily. This forces the opclass or opfamily to be dropped if the + * operator or support func is dropped, and requires the CASCADE option + * to do so. Nor will ALTER OPERATOR FAMILY DROP be allowed. This is + * the right behavior for objects that are essential to an opclass. + * + * If ref_is_hard is false, the entry will have an AUTO dependency on the + * operator or support func, and also an AUTO dependency on the opclass or + * opfamily. This allows ALTER OPERATOR FAMILY DROP, and causes that to + * happen automatically if the operator or support func is dropped. This + * is the right behavior for inessential ("loose") objects. + */ +typedef struct OpFamilyMember +{ + bool is_func; /* is this an operator, or support func? */ + Oid object; /* operator or support func's OID */ + int number; /* strategy or support func number */ + Oid lefttype; /* lefttype */ + Oid righttype; /* righttype */ + Oid sortfamily; /* ordering operator's sort opfamily, or 0 */ + bool ref_is_hard; /* hard or soft dependency? */ + bool ref_is_family; /* is dependency on opclass or opfamily? */ + Oid refobjid; /* OID of opclass or opfamily */ +} OpFamilyMember; + /* * Callback function signatures --- see indexam.sgml for more info. @@ -114,6 +150,12 @@ typedef char *(*ambuildphasename_function) (int64 phasenum); /* validate definition of an opclass for this AM */ typedef bool (*amvalidate_function) (Oid opclassoid); +/* validate operators and support functions to be added to an opclass/family */ +typedef void (*amadjustmembers_function) (Oid opfamilyoid, + Oid opclassoid, + List *operators, + List *functions); + /* prepare for index scan */ typedef IndexScanDesc (*ambeginscan_function) (Relation indexRelation, int nkeys, @@ -224,6 +266,7 @@ typedef struct IndexAmRoutine amproperty_function amproperty; /* can be NULL */ ambuildphasename_function ambuildphasename; /* can be NULL */ amvalidate_function amvalidate; + amadjustmembers_function amadjustmembers; /* can be NULL */ ambeginscan_function ambeginscan; amrescan_function amrescan; amgettuple_function amgettuple; /* can be NULL */ diff --git a/src/include/access/amvalidate.h b/src/include/access/amvalidate.h index f3a0e52d84e..149fc75f856 100644 --- a/src/include/access/amvalidate.h +++ b/src/include/access/amvalidate.h @@ -1,7 +1,8 @@ /*------------------------------------------------------------------------- * * amvalidate.h - * Support routines for index access methods' amvalidate functions. + * Support routines for index access methods' amvalidate and + * amadjustmembers functions. * * Copyright (c) 2016-2020, PostgreSQL Global Development Group * @@ -32,6 +33,8 @@ extern bool check_amproc_signature(Oid funcid, Oid restype, bool exact, extern bool check_amoptsproc_signature(Oid funcid); extern bool check_amop_signature(Oid opno, Oid restype, Oid lefttype, Oid righttype); +extern Oid opclass_for_family_datatype(Oid amoid, Oid opfamilyoid, + Oid datatypeoid); extern bool opfamily_can_sort_type(Oid opfamilyoid, Oid datatypeoid); #endif /* AMVALIDATE_H */ diff --git a/src/include/access/gin_private.h b/src/include/access/gin_private.h index 71eeac205c9..5cb2f72e4cf 100644 --- a/src/include/access/gin_private.h +++ b/src/include/access/gin_private.h @@ -407,6 +407,10 @@ extern ItemPointer ginVacuumItemPointers(GinVacuumState *gvs, /* ginvalidate.c */ extern bool ginvalidate(Oid opclassoid); +extern void ginadjustmembers(Oid opfamilyoid, + Oid opclassoid, + List *operators, + List *functions); /* ginbulk.c */ typedef struct GinEntryAccumulator diff --git a/src/include/access/gist_private.h b/src/include/access/gist_private.h index 4bfc6280002..02e985549f6 100644 --- a/src/include/access/gist_private.h +++ b/src/include/access/gist_private.h @@ -464,6 +464,10 @@ extern bool gistcanreturn(Relation index, int attno); /* gistvalidate.c */ extern bool gistvalidate(Oid opclassoid); +extern void gistadjustmembers(Oid opfamilyoid, + Oid opclassoid, + List *operators, + List *functions); /* gistutil.c */ diff --git a/src/include/access/hash.h b/src/include/access/hash.h index 7e7b1b73d86..bab4d9f1b05 100644 --- a/src/include/access/hash.h +++ b/src/include/access/hash.h @@ -379,6 +379,10 @@ extern IndexBulkDeleteResult *hashvacuumcleanup(IndexVacuumInfo *info, IndexBulkDeleteResult *stats); extern bytea *hashoptions(Datum reloptions, bool validate); extern bool hashvalidate(Oid opclassoid); +extern void hashadjustmembers(Oid opfamilyoid, + Oid opclassoid, + List *operators, + List *functions); /* private routines */ diff --git a/src/include/access/nbtree.h b/src/include/access/nbtree.h index f5274cc7508..65d9698b899 100644 --- a/src/include/access/nbtree.h +++ b/src/include/access/nbtree.h @@ -1141,6 +1141,10 @@ extern bool _bt_allequalimage(Relation rel, bool debugmessage); * prototypes for functions in nbtvalidate.c */ extern bool btvalidate(Oid opclassoid); +extern void btadjustmembers(Oid opfamilyoid, + Oid opclassoid, + List *operators, + List *functions); /* * prototypes for functions in nbtsort.c diff --git a/src/include/access/spgist.h b/src/include/access/spgist.h index 852d1e2961a..9f2ccc1730f 100644 --- a/src/include/access/spgist.h +++ b/src/include/access/spgist.h @@ -220,5 +220,9 @@ extern IndexBulkDeleteResult *spgvacuumcleanup(IndexVacuumInfo *info, /* spgvalidate.c */ extern bool spgvalidate(Oid opclassoid); +extern void spgadjustmembers(Oid opfamilyoid, + Oid opclassoid, + List *operators, + List *functions); #endif /* SPGIST_H */ diff --git a/src/include/catalog/opfam_internal.h b/src/include/catalog/opfam_internal.h deleted file mode 100644 index d63bd9ffa3c..00000000000 --- a/src/include/catalog/opfam_internal.h +++ /dev/null @@ -1,28 +0,0 @@ -/*------------------------------------------------------------------------- - * - * opfam_internal.h - * - * Portions Copyright (c) 1996-2020, PostgreSQL Global Development Group - * Portions Copyright (c) 1994, Regents of the University of California - * - * src/include/catalog/opfam_internal.h - * - *------------------------------------------------------------------------- - */ -#ifndef OPFAM_INTERNAL_H -#define OPFAM_INTERNAL_H - -/* - * We use lists of this struct type to keep track of both operators and - * procedures while building or adding to an opfamily. - */ -typedef struct -{ - Oid object; /* operator or support proc's OID */ - int number; /* strategy or support proc number */ - Oid lefttype; /* lefttype */ - Oid righttype; /* righttype */ - Oid sortfamily; /* ordering operator's sort opfamily, or 0 */ -} OpFamilyMember; - -#endif /* OPFAM_INTERNAL_H */