Refactor some lsyscache routines to eliminate duplicate code and save

a couple of syscache lookups in make_pathkey_from_sortinfo().

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)

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.
@@ -139,40 +139,41 @@ get_opfamily_member(Oid opfamily, Oid lefttype, Oid righttype,
 }
 
 /*
- * get_compare_function_for_ordering_op
- *		Get the OID of the datatype-specific btree comparison function
- *		associated with an ordering operator (a "<" or ">" operator).
+ * 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).
  *
- * *cmpfunc receives the comparison function OID.
- * *reverse is set FALSE if the operator is "<", TRUE if it's ">"
- * (indicating the comparison result must be negated before use).
- *
- * Returns TRUE if successful, FALSE if no btree function can be found.
+ * 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_compare_function_for_ordering_op(Oid opno, Oid *cmpfunc, bool *reverse)
+get_ordering_op_properties(Oid opno,
+						   Oid *opfamily, Oid *opcintype, int16 *strategy)
 {
 	bool		result = false;
 	CatCList   *catlist;
 	int			i;
 
-	/* ensure outputs are set on failure */
-	*cmpfunc = InvalidOid;
-	*reverse = false;
+	/* 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.  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.)
+	 * or ">" operator of any btree opfamily.
 	 */
 	catlist = SearchSysCacheList(AMOPOPID, 1,
 								 ObjectIdGetDatum(opno),
@@ -190,26 +191,65 @@ get_compare_function_for_ordering_op(Oid opno, Oid *cmpfunc, bool *reverse)
 		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);
+			/* 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
+ *		associated with an ordering operator (a "<" or ">" operator).
+ *
+ * *cmpfunc receives the comparison function OID.
+ * *reverse is set FALSE if the operator is "<", TRUE if it's ">"
+ * (indicating the comparison result must be negated before use).
+ *
+ * Returns TRUE if successful, FALSE if no btree function can be found.
+ * (This indicates that the operator is not a valid ordering operator.)
+ */
+bool
+get_compare_function_for_ordering_op(Oid opno, Oid *cmpfunc, bool *reverse)
+{
+	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;
+	return false;
+}
+
 /*
  * get_equality_op_for_ordering_op
  *		Get the OID of the datatype-specific btree equality operator
@@ -222,45 +262,21 @@ Oid
 get_equality_op_for_ordering_op(Oid opno)
 {
 	Oid			result = InvalidOid;
-	CatCList   *catlist;
-	int			i;
-
-	/*
-	 * 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);
+	Oid			opfamily;
+	Oid			opcintype;
+	int16		strategy;
 
-	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)
+	/* Find the operator in pg_amop */
+	if (get_ordering_op_properties(opno,
+								   &opfamily, &opcintype, &strategy))
 	{
 		/* Found a suitable opfamily, get matching equality operator */
-			result = get_opfamily_member(aform->amopfamily,
-										 aform->amoplefttype,
-										 aform->amoprighttype,
+		result = get_opfamily_member(opfamily,
+									 opcintype,
+									 opcintype,
 									 BTEqualStrategyNumber);
-			if (OidIsValid(result))
-				break;
-			/* failure probably shouldn't happen, but keep looking if so */
-		}
 	}
 
-	ReleaseSysCacheList(catlist);
-
 	return result;
 }
 

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);