]> granicus.if.org Git - postgresql/commitdiff
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);