Refactor some lsyscache routines to eliminate duplicate code and save
authorTom Lane <tgl@sss.pgh.pa.us>
Sun, 21 Jan 2007 00:57:15 +0000 (00:57 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Sun, 21 Jan 2007 00:57:15 +0000 (00:57 +0000)
a couple of syscache lookups in make_pathkey_from_sortinfo().

src/backend/optimizer/path/pathkeys.c
src/backend/utils/cache/lsyscache.c
src/include/utils/lsyscache.h

index 15793b4ef99c779e9e5ea48e1705fea9c633a843..37c5e35c23b7d557a1080270f112988ac4489671 100644 (file)
@@ -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)
index 8988d2e10ff7389ec4bee2ada833ec319992f5a1..d2041a507d0eb46feeeca440c0308fe725922d0a 100644 (file)
@@ -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,18 +191,16 @@ 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);
-           result = true;
-           break;
+           /* 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;
+           }
        }
    }
 
@@ -210,6 +209,47 @@ get_compare_function_for_ordering_op(Oid opno, Oid *cmpfunc, bool *reverse)
    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;
+   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;
 }
 
index f0e3f2c20d4b4c7dff6e8178c13672398053ed57..59c690769cba5dedf039ccefcb14bd637618e5cf 100644 (file)
@@ -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);