Change pg_amop's index on (amopclaid,amopopr) to index (amopopr,amopclaid).

tglsfdc · tglsfdc · commit 1c9ac7dfd032 · 2003-05-13T04:38:58.000Z
This makes no difference for existing uses, but allows SelectSortFunction()
and pred_test_simple_clause() to use indexscans instead of seqscans to
locate entries for a particular operator in pg_amop.  Better yet, they can
use the SearchSysCacheList() API to cache the search results.
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
@@ -9,35 +9,30 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/optimizer/path/indxpath.c,v 1.136 2003/03/23 01:49:02 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/optimizer/path/indxpath.c,v 1.137 2003/05/13 04:38:58 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
 #include "postgres.h"
 
 #include <math.h>
 
-#include "access/heapam.h"
 #include "access/nbtree.h"
-#include "catalog/catname.h"
 #include "catalog/pg_amop.h"
 #include "catalog/pg_namespace.h"
 #include "catalog/pg_operator.h"
+#include "catalog/pg_type.h"
 #include "executor/executor.h"
 #include "nodes/makefuncs.h"
-#include "nodes/nodeFuncs.h"
 #include "optimizer/clauses.h"
 #include "optimizer/cost.h"
 #include "optimizer/pathnode.h"
 #include "optimizer/paths.h"
 #include "optimizer/restrictinfo.h"
 #include "optimizer/var.h"
-#include "parser/parse_coerce.h"
-#include "parser/parse_expr.h"
-#include "parser/parse_oper.h"
 #include "rewrite/rewriteManip.h"
 #include "utils/builtins.h"
-#include "utils/fmgroids.h"
+#include "utils/catcache.h"
 #include "utils/lsyscache.h"
 #include "utils/selfuncs.h"
 #include "utils/syscache.h"
@@ -1120,18 +1115,18 @@ pred_test_simple_clause(Expr *predicate, Node *clause)
 				clause_op,
 				test_op;
 	Oid			opclass_id = InvalidOid;
+	bool		found = false;
 	StrategyNumber pred_strategy = 0,
-				clause_strategy,
+				clause_strategy = 0,
 				test_strategy;
 	Expr	   *test_expr;
 	ExprState  *test_exprstate;
 	Datum		test_result;
 	bool		isNull;
-	Relation	relation;
-	HeapScanDesc scan;
-	HeapTuple	tuple;
-	ScanKeyData entry[1];
-	Form_pg_amop aform;
+	HeapTuple	test_tuple;
+	Form_pg_amop test_form;
+	CatCList   *catlist;
+	int			i;
 	EState	   *estate;
 	MemoryContext oldcontext;
 
@@ -1141,7 +1136,8 @@ pred_test_simple_clause(Expr *predicate, Node *clause)
 
 	/*
 	 * Can't do anything more unless they are both binary opclauses with a
-	 * Var on the left and a Const on the right.
+	 * Var on the left and a Const on the right.  (XXX someday try to
+	 * commute Const/Var cases?)
 	 */
 	if (!is_opclause(predicate))
 		return false;
@@ -1174,101 +1170,95 @@ pred_test_simple_clause(Expr *predicate, Node *clause)
 	clause_op = ((OpExpr *) clause)->opno;
 
 	/*
-	 * 1. Find a "btree" strategy number for the pred_op
+	 * 1. Find "btree" strategy numbers for the pred_op and clause_op.
 	 *
-	 * The following assumes that any given operator will only be in a single
-	 * btree operator class.  This is true at least for all the
-	 * pre-defined operator classes.  If it isn't true, then whichever
-	 * operator class happens to be returned first for the given operator
-	 * will be used to find the associated strategy numbers for the test.
-	 * --Nels, Jan '93
+	 * We must find a btree opclass that contains both operators, else the
+	 * implication can't be determined.  If there are multiple such opclasses,
+	 * assume we can use any one to determine the logical relationship of the
+	 * two operators and the correct corresponding test operator.  This should
+	 * work for any logically consistent opclasses.
 	 */
-	ScanKeyEntryInitialize(&entry[0], 0x0,
-						   Anum_pg_amop_amopopr,
-						   F_OIDEQ,
-						   ObjectIdGetDatum(pred_op));
-
-	relation = heap_openr(AccessMethodOperatorRelationName, AccessShareLock);
-	scan = heap_beginscan(relation, SnapshotNow, 1, entry);
+	catlist = SearchSysCacheList(AMOPOPID, 1,
+								 ObjectIdGetDatum(pred_op),
+								 0, 0, 0);
 
-	while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
+	for (i = 0; i < catlist->n_members; i++)
 	{
-		aform = (Form_pg_amop) GETSTRUCT(tuple);
-		if (opclass_is_btree(aform->amopclaid))
+		HeapTuple	pred_tuple = &catlist->members[i]->tuple;
+		Form_pg_amop pred_form = (Form_pg_amop) GETSTRUCT(pred_tuple);
+		HeapTuple	clause_tuple;
+
+		if (!opclass_is_btree(pred_form->amopclaid))
+			continue;
+
+		/* Get the predicate operator's btree strategy number */
+		pred_strategy = (StrategyNumber) pred_form->amopstrategy;
+		Assert(pred_strategy >= 1 && pred_strategy <= 5);
+
+		/*
+		 * Remember which operator class this strategy number came from
+		 */
+		opclass_id = pred_form->amopclaid;
+
+		/*
+		 * From the same opclass, find a strategy num for the clause_op,
+		 * if possible
+		 */
+		clause_tuple = SearchSysCache(AMOPOPID,
+									  ObjectIdGetDatum(clause_op),
+									  ObjectIdGetDatum(opclass_id),
+									  0, 0);
+		if (HeapTupleIsValid(clause_tuple))
 		{
-			/* Get the predicate operator's btree strategy number (1 to 5) */
-			pred_strategy = (StrategyNumber) aform->amopstrategy;
-			Assert(pred_strategy >= 1 && pred_strategy <= 5);
+			Form_pg_amop clause_form = (Form_pg_amop) GETSTRUCT(clause_tuple);
 
-			/*
-			 * Remember which operator class this strategy number came
-			 * from
-			 */
-			opclass_id = aform->amopclaid;
+			/* Get the restriction clause operator's strategy number */
+			clause_strategy = (StrategyNumber) clause_form->amopstrategy;
+			Assert(clause_strategy >= 1 && clause_strategy <= 5);
+
+			ReleaseSysCache(clause_tuple);
+			found = true;
 			break;
 		}
 	}
 
-	heap_endscan(scan);
-	heap_close(relation, AccessShareLock);
-
-	if (!OidIsValid(opclass_id))
-	{
-		/* predicate operator isn't btree-indexable */
-		return false;
-	}
+	ReleaseSysCacheList(catlist);
 
-	/*
-	 * 2. From the same opclass, find a strategy num for the clause_op
-	 */
-	tuple = SearchSysCache(AMOPOPID,
-						   ObjectIdGetDatum(opclass_id),
-						   ObjectIdGetDatum(clause_op),
-						   0, 0);
-	if (!HeapTupleIsValid(tuple))
+	if (!found)
 	{
-		/* clause operator isn't btree-indexable, or isn't in this opclass */
+		/* couldn't find a btree opclass to interpret the operators */
 		return false;
 	}
-	aform = (Form_pg_amop) GETSTRUCT(tuple);
-
-	/* Get the restriction clause operator's strategy number (1 to 5) */
-	clause_strategy = (StrategyNumber) aform->amopstrategy;
-	Assert(clause_strategy >= 1 && clause_strategy <= 5);
-
-	ReleaseSysCache(tuple);
 
 	/*
-	 * 3. Look up the "test" strategy number in the implication table
+	 * 2. Look up the "test" strategy number in the implication table
 	 */
 	test_strategy = BT_implic_table[clause_strategy - 1][pred_strategy - 1];
 	if (test_strategy == 0)
-	{
 		return false;			/* the implication cannot be determined */
-	}
 
 	/*
-	 * 4. From the same opclass, find the operator for the test strategy
+	 * 3. From the same opclass, find the operator for the test strategy
 	 */
-	tuple = SearchSysCache(AMOPSTRATEGY,
-						   ObjectIdGetDatum(opclass_id),
-						   Int16GetDatum(test_strategy),
-						   0, 0);
-	if (!HeapTupleIsValid(tuple))
+	test_tuple = SearchSysCache(AMOPSTRATEGY,
+								ObjectIdGetDatum(opclass_id),
+								Int16GetDatum(test_strategy),
+								0, 0);
+	if (!HeapTupleIsValid(test_tuple))
 	{
-		/* this probably shouldn't fail? */
-		elog(DEBUG1, "pred_test_simple_clause: unknown test_op");
-		return false;
+		/* This should not fail, else pg_amop entry is missing */
+		elog(ERROR, "Missing pg_amop entry for opclass %u strategy %d",
+			 opclass_id, test_strategy);
 	}
-	aform = (Form_pg_amop) GETSTRUCT(tuple);
+	test_form = (Form_pg_amop) GETSTRUCT(test_tuple);
 
 	/* Get the test operator */
-	test_op = aform->amopopr;
+	test_op = test_form->amopopr;
 
-	ReleaseSysCache(tuple);
+	ReleaseSysCache(test_tuple);
 
 	/*
-	 * 5. Evaluate the test.  For this we need an EState.
+	 * 4. Evaluate the test.  For this we need an EState.
 	 */
 	estate = CreateExecutorState();
 
@@ -1298,6 +1288,7 @@ pred_test_simple_clause(Expr *predicate, Node *clause)
 
 	if (isNull)
 	{
+		/* Treat a null result as false ... but it's a tad fishy ... */
 		elog(DEBUG1, "pred_test_simple_clause: null test result");
 		return false;
 	}
diff --git 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
- *	  $Header: /cvsroot/pgsql/src/backend/utils/cache/lsyscache.c,v 1.93 2003/05/09 18:08:48 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/utils/cache/lsyscache.c,v 1.94 2003/05/13 04:38:58 tgl Exp $
  *
  * NOTES
  *	  Eventually, the index information should go through here, too.
@@ -44,8 +44,8 @@ bool
 op_in_opclass(Oid opno, Oid opclass)
 {
 	return SearchSysCacheExists(AMOPOPID,
-								ObjectIdGetDatum(opclass),
 								ObjectIdGetDatum(opno),
+								ObjectIdGetDatum(opclass),
 								0, 0);
 }
 
@@ -67,8 +67,8 @@ op_requires_recheck(Oid opno, Oid opclass)
 	bool		result;
 
 	tp = SearchSysCache(AMOPOPID,
-						ObjectIdGetDatum(opclass),
 						ObjectIdGetDatum(opno),
+						ObjectIdGetDatum(opclass),
 						0, 0);
 	if (!HeapTupleIsValid(tp))
 		elog(ERROR, "op_requires_recheck: op %u is not a member of opclass %u",
diff --git a/src/backend/utils/cache/syscache.c b/src/backend/utils/cache/syscache.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/utils/cache/syscache.c,v 1.87 2002/09/04 20:31:30 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/utils/cache/syscache.c,v 1.88 2003/05/13 04:38:58 tgl Exp $
  *
  * NOTES
  *	  These routines allow the parser/planner/executor to perform
@@ -128,8 +128,8 @@ static const struct cachedesc cacheinfo[] = {
 		0,
 		2,
 		{
-			Anum_pg_amop_amopclaid,
 			Anum_pg_amop_amopopr,
+			Anum_pg_amop_amopclaid,
 			0,
 			0
 	}},
diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c
@@ -78,7 +78,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/utils/sort/tuplesort.c,v 1.32 2002/11/13 00:39:48 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/utils/sort/tuplesort.c,v 1.33 2003/05/13 04:38:58 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -87,18 +87,18 @@
 
 #include "access/heapam.h"
 #include "access/nbtree.h"
-#include "catalog/catname.h"
 #include "catalog/pg_amop.h"
 #include "catalog/pg_amproc.h"
 #include "catalog/pg_operator.h"
 #include "miscadmin.h"
+#include "utils/catcache.h"
 #include "utils/datum.h"
-#include "utils/fmgroids.h"
 #include "utils/logtape.h"
 #include "utils/lsyscache.h"
 #include "utils/syscache.h"
 #include "utils/tuplesort.h"
 
+
 /*
  * Possible states of a Tuplesort object.  These denote the states that
  * persist between calls of Tuplesort routines.
@@ -1708,32 +1708,30 @@ SelectSortFunction(Oid sortOperator,
 				   RegProcedure *sortFunction,
 				   SortFunctionKind *kind)
 {
-	Relation	relation;
-	HeapScanDesc scan;
-	ScanKeyData skey[1];
+	CatCList   *catlist;
+	int			i;
 	HeapTuple	tuple;
 	Form_pg_operator optup;
 	Oid			opclass = InvalidOid;
 
 	/*
-	 * Scan pg_amop to see if the target operator is registered as the "<"
+	 * Search pg_amop to see if the target operator is registered as the "<"
 	 * or ">" operator of any btree opclass.  It's possible that it might
 	 * be registered both ways (eg, if someone were to build a "reverse
 	 * sort" opclass for some reason); prefer the "<" case if so. If the
 	 * operator is registered the same way in multiple opclasses, assume
 	 * we can use the associated comparator function from any one.
 	 */
-	ScanKeyEntryInitialize(&skey[0], 0x0,
-						   Anum_pg_amop_amopopr,
-						   F_OIDEQ,
-						   ObjectIdGetDatum(sortOperator));
+	catlist = SearchSysCacheList(AMOPOPID, 1,
+								 ObjectIdGetDatum(sortOperator),
+								 0, 0, 0);
 
-	relation = heap_openr(AccessMethodOperatorRelationName, AccessShareLock);
-	scan = heap_beginscan(relation, SnapshotNow, 1, skey);
-
-	while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
+	for (i = 0; i < catlist->n_members; i++)
 	{
-		Form_pg_amop aform = (Form_pg_amop) GETSTRUCT(tuple);
+		Form_pg_amop aform;
+
+		tuple = &catlist->members[i]->tuple;
+		aform = (Form_pg_amop) GETSTRUCT(tuple);
 
 		if (!opclass_is_btree(aform->amopclaid))
 			continue;
@@ -1751,8 +1749,7 @@ SelectSortFunction(Oid sortOperator,
 		}
 	}
 
-	heap_endscan(scan);
-	heap_close(relation, AccessShareLock);
+	ReleaseSysCacheList(catlist);
 
 	if (OidIsValid(opclass))
 	{
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
@@ -37,7 +37,7 @@
  * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: catversion.h,v 1.191 2003/05/12 23:08:50 tgl Exp $
+ * $Id: catversion.h,v 1.192 2003/05/13 04:38:58 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -53,6 +53,6 @@
  */
 
 /*							yyyymmddN */
-#define CATALOG_VERSION_NO	200305121
+#define CATALOG_VERSION_NO	200305122
 
 #endif
diff --git a/src/include/catalog/indexing.h b/src/include/catalog/indexing.h

Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,7 @@`
`8`	`8`	`*`
`9`	`9`	`*`
`10`	`10`	`* IDENTIFICATION`
`11`		`- * $Header: /cvsroot/pgsql/src/backend/utils/cache/syscache.c,v 1.87 2002/09/04 20:31:30 momjian Exp $`
	`11`	`+ * $Header: /cvsroot/pgsql/src/backend/utils/cache/syscache.c,v 1.88 2003/05/13 04:38:58 tgl Exp $`
`12`	`12`	`*`
`13`	`13`	`* NOTES`
`14`	`14`	`* These routines allow the parser/planner/executor to perform`
`@@ -128,8 +128,8 @@ static const struct cachedesc cacheinfo[] = {`
`128`	`128`	`0,`
`129`	`129`	`2,`
`130`	`130`	`{`
`131`		`- Anum_pg_amop_amopclaid,`
`132`	`131`	`Anum_pg_amop_amopopr,`
	`132`	`+ Anum_pg_amop_amopclaid,`
`133`	`133`	`0,`
`134`	`134`	`0`
`135`	`135`	`}},`
Original file line number	Diff line number	Diff line change
`@@ -37,7 +37,7 @@`
`37`	`37`	`* Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group`
`38`	`38`	`* Portions Copyright (c) 1994, Regents of the University of California`
`39`	`39`	`*`
`40`		`- * $Id: catversion.h,v 1.191 2003/05/12 23:08:50 tgl Exp $`
	`40`	`+ * $Id: catversion.h,v 1.192 2003/05/13 04:38:58 tgl Exp $`
`41`	`41`	`*`
`42`	`42`	`*-------------------------------------------------------------------------`
`43`	`43`	`*/`
`@@ -53,6 +53,6 @@`
`53`	`53`	`*/`
`54`	`54`
`55`	`55`	`/* yyyymmddN */`
`56`		`-#define CATALOG_VERSION_NO 200305121`
	`56`	`+#define CATALOG_VERSION_NO 200305122`
`57`	`57`
`58`	`58`	`#endif`