diff --git a/src/backend/optimizer/path/pathkeys.c b/src/backend/optimizer/path/pathkeys.c index 15793b4ef9..37c5e35c23 100644 --- a/src/backend/optimizer/path/pathkeys.c +++ b/src/backend/optimizer/path/pathkeys.c @@ -11,7 +11,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/optimizer/path/pathkeys.c,v 1.82 2007/01/20 20:45:39 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/path/pathkeys.c,v 1.83 2007/01/21 00:57:15 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -240,13 +240,11 @@ make_pathkey_from_sortinfo(PlannerInfo *root, bool nulls_first, bool canonicalize) { + Oid opfamily, + opcintype; + int16 strategy; Oid equality_op; List *opfamilies; - Oid opfamily, - lefttype, - righttype; - int strategy; - ListCell *lc; EquivalenceClass *eclass; /* @@ -258,7 +256,17 @@ make_pathkey_from_sortinfo(PlannerInfo *root, * easily be bigger. So, look up the equality operator that goes with * the ordering operator (this should be unique) and get its membership. */ - equality_op = get_equality_op_for_ordering_op(ordering_op); + + /* Find the operator in pg_amop --- failure shouldn't happen */ + if (!get_ordering_op_properties(ordering_op, + &opfamily, &opcintype, &strategy)) + elog(ERROR, "operator %u is not a valid ordering operator", + ordering_op); + /* Get matching equality operator */ + equality_op = get_opfamily_member(opfamily, + opcintype, + opcintype, + BTEqualStrategyNumber); if (!OidIsValid(equality_op)) /* shouldn't happen */ elog(ERROR, "could not find equality operator for ordering operator %u", ordering_op); @@ -267,33 +275,8 @@ make_pathkey_from_sortinfo(PlannerInfo *root, elog(ERROR, "could not find opfamilies for ordering operator %u", ordering_op); - /* - * Next we have to determine the strategy number to put into the pathkey. - * In the presence of reverse-sort opclasses there might be two answers. - * We prefer the one associated with the first opfamilies member that - * this ordering_op appears in (this will be consistently defined in - * normal system operation; see comments for get_mergejoin_opfamilies()). - */ - opfamily = InvalidOid; - strategy = 0; - foreach(lc, opfamilies) - { - opfamily = lfirst_oid(lc); - strategy = get_op_opfamily_strategy(ordering_op, opfamily); - if (strategy) - break; - } - if (!(strategy == BTLessStrategyNumber || - strategy == BTGreaterStrategyNumber)) - elog(ERROR, "ordering operator %u is has wrong strategy number %d", - ordering_op, strategy); - - /* Need the declared input type of the operator, too */ - op_input_types(ordering_op, &lefttype, &righttype); - Assert(lefttype == righttype); - /* Now find or create a matching EquivalenceClass */ - eclass = get_eclass_for_sort_expr(root, expr, lefttype, opfamilies); + eclass = get_eclass_for_sort_expr(root, expr, opcintype, opfamilies); /* And finally we can find or create a PathKey node */ if (canonicalize) diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c index 8988d2e10f..d2041a507d 100644 --- a/src/backend/utils/cache/lsyscache.c +++ b/src/backend/utils/cache/lsyscache.c @@ -7,7 +7,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/utils/cache/lsyscache.c,v 1.144 2007/01/20 20:45:40 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/utils/cache/lsyscache.c,v 1.145 2007/01/21 00:57:15 tgl Exp $ * * NOTES * Eventually, the index information should go through here, too. @@ -138,6 +138,77 @@ get_opfamily_member(Oid opfamily, Oid lefttype, Oid righttype, return result; } +/* + * get_ordering_op_properties + * Given the OID of an ordering operator (a btree "<" or ">" operator), + * determine its opfamily, its declared input datatype, and its + * strategy number (BTLessStrategyNumber or BTGreaterStrategyNumber). + * + * Returns TRUE if successful, FALSE if no matching pg_amop entry exists. + * (This indicates that the operator is not a valid ordering operator.) + * + * Note: the operator could be registered in multiple families, for example + * if someone were to build a "reverse sort" opfamily. This would result in + * uncertainty as to whether "ORDER BY USING op" would default to NULLS FIRST + * or NULLS LAST, as well as inefficient planning due to failure to match up + * pathkeys that should be the same. So we want a determinate result here. + * Because of the way the syscache search works, we'll use the interpretation + * associated with the opfamily with smallest OID, which is probably + * determinate enough. Since there is no longer any particularly good reason + * to build reverse-sort opfamilies, it doesn't seem worth expending any + * additional effort on ensuring consistency. + */ +bool +get_ordering_op_properties(Oid opno, + Oid *opfamily, Oid *opcintype, int16 *strategy) +{ + bool result = false; + CatCList *catlist; + int i; + + /* ensure outputs are initialized on failure */ + *opfamily = InvalidOid; + *opcintype = InvalidOid; + *strategy = 0; + + /* + * Search pg_amop to see if the target operator is registered as the "<" + * or ">" operator of any btree opfamily. + */ + catlist = SearchSysCacheList(AMOPOPID, 1, + ObjectIdGetDatum(opno), + 0, 0, 0); + + for (i = 0; i < catlist->n_members; i++) + { + HeapTuple tuple = &catlist->members[i]->tuple; + Form_pg_amop aform = (Form_pg_amop) GETSTRUCT(tuple); + + /* must be btree */ + if (aform->amopmethod != BTREE_AM_OID) + continue; + + if (aform->amopstrategy == BTLessStrategyNumber || + aform->amopstrategy == BTGreaterStrategyNumber) + { + /* Found it ... should have consistent input types */ + if (aform->amoplefttype == aform->amoprighttype) + { + /* Found a suitable opfamily, return info */ + *opfamily = aform->amopfamily; + *opcintype = aform->amoplefttype; + *strategy = aform->amopstrategy; + result = true; + break; + } + } + } + + ReleaseSysCacheList(catlist); + + return result; +} + /* * get_compare_function_for_ordering_op * Get the OID of the datatype-specific btree comparison function @@ -153,61 +224,30 @@ get_opfamily_member(Oid opfamily, Oid lefttype, Oid righttype, bool get_compare_function_for_ordering_op(Oid opno, Oid *cmpfunc, bool *reverse) { - bool result = false; - CatCList *catlist; - int i; + Oid opfamily; + Oid opcintype; + int16 strategy; + + /* Find the operator in pg_amop */ + if (get_ordering_op_properties(opno, + &opfamily, &opcintype, &strategy)) + { + /* Found a suitable opfamily, get matching support function */ + *cmpfunc = get_opfamily_proc(opfamily, + opcintype, + opcintype, + BTORDER_PROC); + if (!OidIsValid(*cmpfunc)) /* should not happen */ + elog(ERROR, "missing support function %d(%u,%u) in opfamily %u", + BTORDER_PROC, opcintype, opcintype, opfamily); + *reverse = (strategy == BTGreaterStrategyNumber); + return true; + } /* ensure outputs are set on failure */ *cmpfunc = InvalidOid; *reverse = false; - - /* - * Search pg_amop to see if the target operator is registered as the "<" - * or ">" operator of any btree opfamily. It's possible that it might be - * registered both ways (if someone were to build a "reverse sort" - * opfamily); assume we can use either interpretation. (Note: the - * existence of a reverse-sort opfamily would result in uncertainty as - * to whether "ORDER BY USING op" would default to NULLS FIRST or NULLS - * LAST. Since there is no longer any particularly good reason to build - * reverse-sort opfamilies, we don't bother expending any extra work to - * make this more determinate. In practice, because of the way the - * syscache search works, we'll use the interpretation associated with - * the opfamily with smallest OID, which is probably determinate enough.) - */ - catlist = SearchSysCacheList(AMOPOPID, 1, - ObjectIdGetDatum(opno), - 0, 0, 0); - - for (i = 0; i < catlist->n_members; i++) - { - HeapTuple tuple = &catlist->members[i]->tuple; - Form_pg_amop aform = (Form_pg_amop) GETSTRUCT(tuple); - - /* must be btree */ - if (aform->amopmethod != BTREE_AM_OID) - continue; - - if (aform->amopstrategy == BTLessStrategyNumber || - aform->amopstrategy == BTGreaterStrategyNumber) - { - /* Found a suitable opfamily, get matching support function */ - *reverse = (aform->amopstrategy == BTGreaterStrategyNumber); - *cmpfunc = get_opfamily_proc(aform->amopfamily, - aform->amoplefttype, - aform->amoprighttype, - BTORDER_PROC); - if (!OidIsValid(*cmpfunc)) /* should not happen */ - elog(ERROR, "missing support function %d(%u,%u) in opfamily %u", - BTORDER_PROC, aform->amoplefttype, aform->amoprighttype, - aform->amopfamily); - result = true; - break; - } - } - - ReleaseSysCacheList(catlist); - - return result; + return false; } /* @@ -222,45 +262,21 @@ Oid get_equality_op_for_ordering_op(Oid opno) { Oid result = InvalidOid; - CatCList *catlist; - int i; + Oid opfamily; + Oid opcintype; + int16 strategy; - /* - * Search pg_amop to see if the target operator is registered as the "<" - * or ">" operator of any btree opfamily. This is exactly like - * get_compare_function_for_ordering_op except we don't care whether the - * ordering op is "<" or ">" ... the equality operator will be the same - * either way. - */ - catlist = SearchSysCacheList(AMOPOPID, 1, - ObjectIdGetDatum(opno), - 0, 0, 0); - - for (i = 0; i < catlist->n_members; i++) + /* Find the operator in pg_amop */ + if (get_ordering_op_properties(opno, + &opfamily, &opcintype, &strategy)) { - HeapTuple tuple = &catlist->members[i]->tuple; - Form_pg_amop aform = (Form_pg_amop) GETSTRUCT(tuple); - - /* must be btree */ - if (aform->amopmethod != BTREE_AM_OID) - continue; - - if (aform->amopstrategy == BTLessStrategyNumber || - aform->amopstrategy == BTGreaterStrategyNumber) - { - /* Found a suitable opfamily, get matching equality operator */ - result = get_opfamily_member(aform->amopfamily, - aform->amoplefttype, - aform->amoprighttype, - BTEqualStrategyNumber); - if (OidIsValid(result)) - break; - /* failure probably shouldn't happen, but keep looking if so */ - } + /* Found a suitable opfamily, get matching equality operator */ + result = get_opfamily_member(opfamily, + opcintype, + opcintype, + BTEqualStrategyNumber); } - ReleaseSysCacheList(catlist); - return result; } diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h index f0e3f2c20d..59c690769c 100644 --- a/src/include/utils/lsyscache.h +++ b/src/include/utils/lsyscache.h @@ -6,7 +6,7 @@ * Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/utils/lsyscache.h,v 1.113 2007/01/20 20:45:41 tgl Exp $ + * $PostgreSQL: pgsql/src/include/utils/lsyscache.h,v 1.114 2007/01/21 00:57:15 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -35,6 +35,8 @@ extern void get_op_opfamily_properties(Oid opno, Oid opfamily, bool *recheck); extern Oid get_opfamily_member(Oid opfamily, Oid lefttype, Oid righttype, int16 strategy); +extern bool get_ordering_op_properties(Oid opno, + Oid *opfamily, Oid *opcintype, int16 *strategy); extern bool get_compare_function_for_ordering_op(Oid opno, Oid *cmpfunc, bool *reverse); extern Oid get_equality_op_for_ordering_op(Oid opno);