From 066926dfbb49c7684bf51153c71634d05f1244ce Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sun, 21 Jan 2007 00:57:15 +0000
Subject: [PATCH] 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 | 49 +++-----
src/backend/utils/cache/lsyscache.c | 156 ++++++++++++++------------
src/include/utils/lsyscache.h | 4 +-
3 files changed, 105 insertions(+), 104 deletions(-)
diff --git a/src/backend/optimizer/path/pathkeys.c b/src/backend/optimizer/path/pathkeys.c
index 15793b4ef99..37c5e35c23b 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 8988d2e10ff..d2041a507d0 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.
@@ -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;
}
diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h
index f0e3f2c20d4..59c690769cb 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);
--
GitLab