Simplify and speed up mapping of index opfamilies to pathkeys.

Formerly we looked up the operators associated with each index (caching
them in relcache) and then the planner looked up the btree opfamily
containing such operators in order to build the btree-centric pathkey
representation that describes the index's sort order.  This is quite
pointless for btree indexes: we might as well just use the index's opfamily
information directly.  That saves syscache lookup cycles during planning,
and furthermore allows us to eliminate the relcache's caching of operators
altogether, which may help in reducing backend startup time.

I added code to plancat.c to perform the same type of double lookup
on-the-fly if it's ever faced with a non-btree amcanorder index AM.
If such a thing actually becomes interesting for production, we should
replace that logic with some more-direct method for identifying the
corresponding btree opfamily; but it's not worth spending effort on now.

There is considerably more to do pursuant to my recent proposal to get rid
of sort-operator-based representations of sort orderings, but this patch
grabs some of the low-hanging fruit.  I'll look at the remainder of that
work after the current commitfest.

src/backend/optimizer/path/indxpath.c

@@ -380,7 +380,7 @@ find_usable_indexes(PlannerInfo *root, RelOptInfo *rel,
 		 * how many of them are actually useful for this query.  This is not
 		 * relevant unless we are at top level.
 		 */
-		index_is_ordered = OidIsValid(index->fwdsortop[0]);
+		index_is_ordered = (index->sortopfamily != NULL);
 		if (index_is_ordered && possibly_useful_pathkeys &&
 			istoplevel && outer_rel == NULL)
 		{

src/backend/optimizer/path/pathkeys.c

@@ -36,12 +36,6 @@ static PathKey *make_canonical_pathkey(PlannerInfo *root,
 					   EquivalenceClass *eclass, Oid opfamily,
 					   int strategy, bool nulls_first);
 static bool pathkey_is_redundant(PathKey *new_pathkey, List *pathkeys);
-static PathKey *make_pathkey_from_sortinfo(PlannerInfo *root,
-						   Expr *expr, Oid ordering_op,
-						   bool nulls_first,
-						   Index sortref,
-						   bool create_it,
-						   bool canonicalize);
 static Var *find_indexkey_var(PlannerInfo *root, RelOptInfo *rel,
 				  AttrNumber varattno);
 static bool right_merge_direction(PlannerInfo *root, PathKey *pathkey);
@@ -224,9 +218,9 @@ canonicalize_pathkeys(PlannerInfo *root, List *pathkeys)
 
 /*
  * make_pathkey_from_sortinfo
- *	  Given an expression, a sortop, and a nulls-first flag, create
- *	  a PathKey.  If canonicalize = true, the result is a "canonical"
- *	  PathKey, otherwise not.  (But note it might be redundant anyway.)
+ *	  Given an expression and sort-order information, create a PathKey.
+ *	  If canonicalize = true, the result is a "canonical" PathKey,
+ *	  otherwise not.  (But note it might be redundant anyway.)
  *
  * If the PathKey is being generated from a SortGroupClause, sortref should be
  * the SortGroupClause's SortGroupRef; otherwise zero.
@@ -240,46 +234,39 @@ canonicalize_pathkeys(PlannerInfo *root, List *pathkeys)
  */
 static PathKey *
 make_pathkey_from_sortinfo(PlannerInfo *root,
-						   Expr *expr, Oid ordering_op,
+						   Expr *expr,
+						   Oid opfamily,
+						   Oid opcintype,
+						   bool reverse_sort,
 						   bool nulls_first,
 						   Index sortref,
 						   bool create_it,
 						   bool canonicalize)
 {
-	Oid			opfamily,
-				opcintype;
 	int16		strategy;
 	Oid			equality_op;
 	List	   *opfamilies;
 	EquivalenceClass *eclass;
 
+	strategy = reverse_sort ? BTGreaterStrategyNumber : BTLessStrategyNumber;
+
 	/*
-	 * An ordering operator fully determines the behavior of its opfamily, so
-	 * could only meaningfully appear in one family --- or perhaps two if one
-	 * builds a reverse-sort opfamily, but there's not much point in that
-	 * anymore.  But EquivalenceClasses need to contain opfamily lists based
-	 * on the family membership of equality operators, which could easily be
-	 * bigger.	So, look up the equality operator that goes with the ordering
-	 * operator (this should be unique) and get its membership.
+	 * EquivalenceClasses need to contain opfamily lists based on the family
+	 * membership of mergejoinable equality operators, which could belong to
+	 * more than one opfamily.  So we have to look up the opfamily's equality
+	 * operator and get its membership.
 	 */
-
-	/* 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);
+		elog(ERROR, "could not find equality operator for opfamily %u",
+			 opfamily);
 	opfamilies = get_mergejoin_opfamilies(equality_op);
 	if (!opfamilies)			/* certainly should find some */
-		elog(ERROR, "could not find opfamilies for ordering operator %u",
-			 ordering_op);
+		elog(ERROR, "could not find opfamilies for equality operator %u",
+			 equality_op);
 
 	/*
 	 * When dealing with binary-compatible opclasses, we have to ensure that
@@ -322,6 +309,42 @@ make_pathkey_from_sortinfo(PlannerInfo *root,
 		return makePathKey(eclass, opfamily, strategy, nulls_first);
 }
 
+/*
+ * make_pathkey_from_sortop
+ *	  Like make_pathkey_from_sortinfo, but work from a sort operator.
+ *
+ * This should eventually go away, but we need to restructure SortGroupClause
+ * first.
+ */
+static PathKey *
+make_pathkey_from_sortop(PlannerInfo *root,
+						 Expr *expr,
+						 Oid ordering_op,
+						 bool nulls_first,
+						 Index sortref,
+						 bool create_it,
+						 bool canonicalize)
+{
+	Oid			opfamily,
+				opcintype;
+	int16		strategy;
+
+	/* 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);
+	return make_pathkey_from_sortinfo(root,
+									  expr,
+									  opfamily,
+									  opcintype,
+									  (strategy == BTGreaterStrategyNumber),
+									  nulls_first,
+									  sortref,
+									  create_it,
+									  canonicalize);
+}
+
 
 /****************************************************************************
  *		PATHKEY COMPARISONS
@@ -479,11 +502,10 @@ get_cheapest_fractional_path_for_pathkeys(List *paths,
  * build_index_pathkeys
  *	  Build a pathkeys list that describes the ordering induced by an index
  *	  scan using the given index.  (Note that an unordered index doesn't
- *	  induce any ordering; such an index will have no sortop OIDS in
- *	  its sortops arrays, and we will return NIL.)
+ *	  induce any ordering, so we return NIL.)
  *
- * If 'scandir' is BackwardScanDirection, attempt to build pathkeys
- * representing a backwards scan of the index.	Return NIL if can't do it.
+ * If 'scandir' is BackwardScanDirection, build pathkeys representing a
+ * backwards scan of the index.
  *
  * The result is canonical, meaning that redundant pathkeys are removed;
  * it may therefore have fewer entries than there are index columns.
@@ -500,12 +522,16 @@ build_index_pathkeys(PlannerInfo *root,
 					 ScanDirection scandir)
 {
 	List	   *retval = NIL;
-	ListCell   *indexprs_item = list_head(index->indexprs);
+	ListCell   *indexprs_item;
 	int			i;
 
+	if (index->sortopfamily == NULL)
+		return NIL;				/* non-orderable index */
+
+	indexprs_item = list_head(index->indexprs);
 	for (i = 0; i < index->ncolumns; i++)
 	{
-		Oid			sortop;
+		bool		reverse_sort;
 		bool		nulls_first;
 		int			ikey;
 		Expr	   *indexkey;
@@ -513,18 +539,15 @@ build_index_pathkeys(PlannerInfo *root,
 
 		if (ScanDirectionIsBackward(scandir))
 		{
-			sortop = index->revsortop[i];
+			reverse_sort = !index->reverse_sort[i];
 			nulls_first = !index->nulls_first[i];
 		}
 		else
 		{
-			sortop = index->fwdsortop[i];
+			reverse_sort = index->reverse_sort[i];
 			nulls_first = index->nulls_first[i];
 		}
 
-		if (!OidIsValid(sortop))
-			break;				/* no more orderable columns */
-
 		ikey = index->indexkeys[i];
 		if (ikey != 0)
 		{
@@ -543,7 +566,9 @@ build_index_pathkeys(PlannerInfo *root,
 		/* OK, try to make a canonical pathkey for this sort key */
 		cpathkey = make_pathkey_from_sortinfo(root,
 											  indexkey,
-											  sortop,
+											  index->sortopfamily[i],
+											  index->opcintype[i],
+											  reverse_sort,
 											  nulls_first,
 											  0,
 											  false,
@@ -892,13 +917,13 @@ make_pathkeys_for_sortclauses(PlannerInfo *root,
 
 		sortkey = (Expr *) get_sortgroupclause_expr(sortcl, tlist);
 		Assert(OidIsValid(sortcl->sortop));
-		pathkey = make_pathkey_from_sortinfo(root,
-											 sortkey,
-											 sortcl->sortop,
-											 sortcl->nulls_first,
-											 sortcl->tleSortGroupRef,
-											 true,
-											 canonicalize);
+		pathkey = make_pathkey_from_sortop(root,
+										   sortkey,
+										   sortcl->sortop,
+										   sortcl->nulls_first,
+										   sortcl->tleSortGroupRef,
+										   true,
+										   canonicalize);
 
 		/* Canonical form eliminates redundant ordering keys */
 		if (canonicalize)
@@ -935,13 +960,13 @@ make_pathkeys_for_aggregate(PlannerInfo *root,
 	 * We arbitrarily set nulls_first to false.  Actually, a MIN/MAX agg can
 	 * use either nulls ordering option, but that is dealt with elsewhere.
 	 */
-	pathkey = make_pathkey_from_sortinfo(root,
-										 aggtarget,
-										 aggsortop,
-										 false,	/* nulls_first */
-										 0,
-										 true,
-										 false);
+	pathkey = make_pathkey_from_sortop(root,
+									   aggtarget,
+									   aggsortop,
+									   false,	/* nulls_first */
+									   0,
+									   true,
+									   false);
 	return list_make1(pathkey);
 }
 

src/backend/optimizer/util/plancat.c

@@ -189,19 +189,9 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
 				RelationGetForm(indexRelation)->reltablespace;
 			info->rel = rel;
 			info->ncolumns = ncolumns = index->indnatts;
-
-			/*
-			 * Allocate per-column info arrays.  To save a few palloc cycles
-			 * we allocate all the Oid-type arrays in one request.  We must
-			 * pre-zero the sortop and nulls_first arrays in case the index is
-			 * unordered.
-			 */
 			info->indexkeys = (int *) palloc(sizeof(int) * ncolumns);
-			info->opfamily = (Oid *) palloc0(sizeof(Oid) * (4 * ncolumns));
-			info->opcintype = info->opfamily + ncolumns;
-			info->fwdsortop = info->opcintype + ncolumns;
-			info->revsortop = info->fwdsortop + ncolumns;
-			info->nulls_first = (bool *) palloc0(sizeof(bool) * ncolumns);
+			info->opfamily = (Oid *) palloc(sizeof(Oid) * ncolumns);
+			info->opcintype = (Oid *) palloc(sizeof(Oid) * ncolumns);
 
 			for (i = 0; i < ncolumns; i++)
 			{
@@ -219,49 +209,90 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
 			info->amhasgetbitmap = OidIsValid(indexRelation->rd_am->amgetbitmap);
 
 			/*
-			 * Fetch the ordering operators associated with the index, if any.
-			 * We expect that all ordering-capable indexes use btree's
-			 * strategy numbers for the ordering operators.
+			 * Fetch the ordering information for the index, if any.
 			 */
-			if (indexRelation->rd_am->amcanorder)
+			if (info->relam == BTREE_AM_OID)
 			{
-				int			nstrat = indexRelation->rd_am->amstrategies;
+				/*
+				 * If it's a btree index, we can use its opfamily OIDs
+				 * directly as the sort ordering opfamily OIDs.
+				 */
+				Assert(indexRelation->rd_am->amcanorder);
+
+				info->sortopfamily = info->opfamily;
+				info->reverse_sort = (bool *) palloc(sizeof(bool) * ncolumns);
+				info->nulls_first = (bool *) palloc(sizeof(bool) * ncolumns);
 
 				for (i = 0; i < ncolumns; i++)
 				{
 					int16		opt = indexRelation->rd_indoption[i];
-					int			fwdstrat;
-					int			revstrat;
 
-					if (opt & INDOPTION_DESC)
-					{
-						fwdstrat = BTGreaterStrategyNumber;
-						revstrat = BTLessStrategyNumber;
-					}
-					else
-					{
-						fwdstrat = BTLessStrategyNumber;
-						revstrat = BTGreaterStrategyNumber;
-					}
+					info->reverse_sort[i] = (opt & INDOPTION_DESC) != 0;
+					info->nulls_first[i] = (opt & INDOPTION_NULLS_FIRST) != 0;
+				}
+			}
+			else if (indexRelation->rd_am->amcanorder)
+			{
+				/*
+				 * Otherwise, identify the corresponding btree opfamilies by
+				 * trying to map this index's "<" operators into btree.  Since
+				 * "<" uniquely defines the behavior of a sort order, this is
+				 * a sufficient test.
+				 *
+				 * XXX This method is rather slow and also requires the
+				 * undesirable assumption that the other index AM numbers its
+				 * strategies the same as btree.  It'd be better to have a way
+				 * to explicitly declare the corresponding btree opfamily for
+				 * each opfamily of the other index type.  But given the lack
+				 * of current or foreseeable amcanorder index types, it's not
+				 * worth expending more effort on now.
+				 */
+				info->sortopfamily = (Oid *) palloc(sizeof(Oid) * ncolumns);
+				info->reverse_sort = (bool *) palloc(sizeof(bool) * ncolumns);
+				info->nulls_first = (bool *) palloc(sizeof(bool) * ncolumns);
+
+				for (i = 0; i < ncolumns; i++)
+				{
+					int16		opt = indexRelation->rd_indoption[i];
+					Oid			ltopr;
+					Oid			btopfamily;
+					Oid			btopcintype;
+					int16		btstrategy;
 
-					/*
-					 * Index AM must have a fixed set of strategies for it to
-					 * make sense to specify amcanorder, so we need not allow
-					 * the case amstrategies == 0.
-					 */
-					if (fwdstrat > 0)
+					info->reverse_sort[i] = (opt & INDOPTION_DESC) != 0;
+					info->nulls_first[i] = (opt & INDOPTION_NULLS_FIRST) != 0;
+
+					ltopr = get_opfamily_member(info->opfamily[i],
+												info->opcintype[i],
+												info->opcintype[i],
+												BTLessStrategyNumber);
+					if (OidIsValid(ltopr) &&
+						get_ordering_op_properties(ltopr,
+												   &btopfamily,
+												   &btopcintype,
+												   &btstrategy) &&
+						btopcintype == info->opcintype[i] &&
+						btstrategy == BTLessStrategyNumber)
 					{
-						Assert(fwdstrat <= nstrat);
-						info->fwdsortop[i] = indexRelation->rd_operator[i * nstrat + fwdstrat - 1];
+						/* Successful mapping */
+						info->sortopfamily[i] = btopfamily;
 					}
-					if (revstrat > 0)
+					else
 					{
-						Assert(revstrat <= nstrat);
-						info->revsortop[i] = indexRelation->rd_operator[i * nstrat + revstrat - 1];
+						/* Fail ... quietly treat index as unordered */
+						info->sortopfamily = NULL;
+						info->reverse_sort = NULL;
+						info->nulls_first = NULL;
+						break;
 					}
-					info->nulls_first[i] = (opt & INDOPTION_NULLS_FIRST) != 0;
 				}
 			}
+			else
+			{
+				info->sortopfamily = NULL;
+				info->reverse_sort = NULL;
+				info->nulls_first = NULL;
+			}
 
 			/*
 			 * Fetch the index expressions and predicate, if any.  We must

src/backend/utils/adt/selfuncs.c

@@ -4567,14 +4567,26 @@ get_actual_variable_range(PlannerInfo *root, VariableStatData *vardata,
 		 * The first index column must match the desired variable and sort
 		 * operator --- but we can use a descending-order index.
 		 */
-		if (sortop == index->fwdsortop[0])
-			indexscandir = ForwardScanDirection;
-		else if (sortop == index->revsortop[0])
-			indexscandir = BackwardScanDirection;
-		else
-			continue;
 		if (!match_index_to_operand(vardata->var, 0, index))
 			continue;
+		switch (get_op_opfamily_strategy(sortop, index->sortopfamily[0]))
+		{
+			case BTLessStrategyNumber:
+				if (index->reverse_sort[0])
+					indexscandir = BackwardScanDirection;
+				else
+					indexscandir = ForwardScanDirection;
+				break;
+			case BTGreaterStrategyNumber:
+				if (index->reverse_sort[0])
+					indexscandir = ForwardScanDirection;
+				else
+					indexscandir = BackwardScanDirection;
+				break;
+			default:
+				/* index doesn't match the sortop */
+				continue;
+		}
 
 		/*
 		 * Found a suitable index to extract data from.  We'll need an EState
@@ -6150,12 +6162,18 @@ btcostestimate(PG_FUNCTION_ARGS)
 
 	if (HeapTupleIsValid(vardata.statsTuple))
 	{
+		Oid			sortop;
 		float4	   *numbers;
 		int			nnumbers;
 
-		if (get_attstatsslot(vardata.statsTuple, InvalidOid, 0,
+		sortop = get_opfamily_member(index->opfamily[0],
+									 index->opcintype[0],
+									 index->opcintype[0],
+									 BTLessStrategyNumber);
+		if (OidIsValid(sortop) &&
+			get_attstatsslot(vardata.statsTuple, InvalidOid, 0,
 							 STATISTIC_KIND_CORRELATION,
-							 index->fwdsortop[0],
+							 sortop,
 							 NULL,
 							 NULL, NULL,
 							 &numbers, &nnumbers))
@@ -6165,6 +6183,9 @@ btcostestimate(PG_FUNCTION_ARGS)
 			Assert(nnumbers == 1);
 			varCorrelation = numbers[0];
 
+			if (index->reverse_sort[0])
+				varCorrelation = -varCorrelation;
+
 			if (index->ncolumns > 1)
 				*indexCorrelation = varCorrelation * 0.75;
 			else
@@ -6172,25 +6193,6 @@ btcostestimate(PG_FUNCTION_ARGS)
 
 			free_attstatsslot(InvalidOid, NULL, 0, numbers, nnumbers);
 		}
-		else if (get_attstatsslot(vardata.statsTuple, InvalidOid, 0,
-								  STATISTIC_KIND_CORRELATION,
-								  index->revsortop[0],
-								  NULL,
-								  NULL, NULL,
-								  &numbers, &nnumbers))
-		{
-			double		varCorrelation;
-
-			Assert(nnumbers == 1);
-			varCorrelation = numbers[0];
-
-			if (index->ncolumns > 1)
-				*indexCorrelation = -varCorrelation * 0.75;
-			else
-				*indexCorrelation = -varCorrelation;
-
-			free_attstatsslot(InvalidOid, NULL, 0, numbers, nnumbers);
-		}
 	}
 
 	ReleaseVariableStats(vardata);

src/include/nodes/relation.h

@@ -421,20 +421,17 @@ typedef struct RelOptInfo
  * IndexOptInfo
  *		Per-index information for planning/optimization
  *
- *		Prior to Postgres 7.0, RelOptInfo was used to describe both relations
- *		and indexes, but that created confusion without actually doing anything
- *		useful.  So now we have a separate IndexOptInfo struct for indexes.
- *
- *		opfamily[], indexkeys[], opcintype[], fwdsortop[], revsortop[],
- *		and nulls_first[] each have ncolumns entries.
+ *		opfamily[], indexkeys[], and opcintype[] each have ncolumns entries.
+ *		sortopfamily[], reverse_sort[], and nulls_first[] likewise have
+ *		ncolumns entries, if the index is ordered; but if it is unordered,
+ *		those pointers are NULL.
  *
  *		Zeroes in the indexkeys[] array indicate index columns that are
  *		expressions; there is one element in indexprs for each such column.
  *
- *		For an unordered index, the sortop arrays contains zeroes.	Note that
- *		fwdsortop[] and nulls_first[] describe the sort ordering of a forward
- *		indexscan; we can also consider a backward indexscan, which will
- *		generate sort order described by revsortop/!nulls_first.
+ *		For an ordered index, reverse_sort[] and nulls_first[] describe the
+ *		sort ordering of a forward indexscan; we can also consider a backward
+ *		indexscan, which will generate the reverse ordering.
  *
  *		The indexprs and indpred expressions have been run through
  *		prepqual.c and eval_const_expressions() for ease of matching to
@@ -457,8 +454,8 @@ typedef struct IndexOptInfo
 	Oid		   *opfamily;		/* OIDs of operator families for columns */
 	int		   *indexkeys;		/* column numbers of index's keys, or 0 */
 	Oid		   *opcintype;		/* OIDs of opclass declared input data types */
-	Oid		   *fwdsortop;		/* OIDs of sort operators for each column */
-	Oid		   *revsortop;		/* OIDs of sort operators for backward scan */
+	Oid		   *sortopfamily;	/* OIDs of btree opfamilies, if orderable */
+	bool	   *reverse_sort;	/* is sort order descending? */
 	bool	   *nulls_first;	/* do NULLs come first in the sort order? */
 	Oid			relam;			/* OID of the access method (in pg_am) */
 

src/include/utils/rel.h

@@ -178,10 +178,10 @@ typedef struct RelationData
 	/*
 	 * index access support info (used only for an index relation)
 	 *
-	 * Note: only default operators and support procs for each opclass are
-	 * cached, namely those with lefttype and righttype equal to the opclass's
-	 * opcintype.  The arrays are indexed by strategy or support number, which
-	 * is a sufficient identifier given that restriction.
+	 * Note: only default support procs for each opclass are cached, namely
+	 * those with lefttype and righttype equal to the opclass's opcintype.
+	 * The arrays are indexed by support function number, which is a
+	 * sufficient identifier given that restriction.
 	 *
 	 * Note: rd_amcache is available for index AMs to cache private data about
 	 * an index.  This must be just a cache since it may get reset at any time
@@ -194,7 +194,6 @@ typedef struct RelationData
 	RelationAmInfo *rd_aminfo;	/* lookup info for funcs found in pg_am */
 	Oid		   *rd_opfamily;	/* OIDs of op families for each index col */
 	Oid		   *rd_opcintype;	/* OIDs of opclass declared input data types */
-	Oid		   *rd_operator;	/* OIDs of index operators */
 	RegProcedure *rd_support;	/* OIDs of support procedures */
 	FmgrInfo   *rd_supportinfo; /* lookup info for support procedures */
 	int16	   *rd_indoption;	/* per-column AM-specific flags */