Remove assumptions that not-equals operators cannot be in any opclass.

get_op_btree_interpretation assumed this in order to save some duplication
of code, but it's not true in general anymore because we added <> support
to btree_gist.  (We still assume it for btree opclasses, though.)

Also, essentially the same logic was baked into predtest.c.  Get rid of
that duplication by generalizing get_op_btree_interpretation so that it
can be used by predtest.c.

Per bug report from Denis de Bernardy and investigation by Jeff Davis,
though I didn't use Jeff's patch exactly as-is.

Back-patch to 9.1; we do not support this usage before that.

src/backend/optimizer/util/predtest.c

@@ -1250,6 +1250,7 @@ list_member_strip(List *list, Expr *datum)
  * and in addition we use (6) to represent <>.	<> is not a btree-indexable
  * operator, but we assume here that if an equality operator of a btree
  * opfamily has a negator operator, the negator behaves as <> for the opfamily.
+ * (This convention is also known to get_op_btree_interpretation().)
  *
  * The interpretation of:
  *
@@ -1286,7 +1287,7 @@ list_member_strip(List *list, Expr *datum)
 #define BTEQ BTEqualStrategyNumber
 #define BTGE BTGreaterEqualStrategyNumber
 #define BTGT BTGreaterStrategyNumber
-#define BTNE 6
+#define BTNE ROWCOMPARE_NE
 
 static const StrategyNumber BT_implic_table[6][6] = {
 /*
@@ -1557,18 +1558,12 @@ get_btree_test_op(Oid pred_op, Oid clause_op, bool refute_it)
 	OprProofCacheKey key;
 	OprProofCacheEntry *cache_entry;
 	bool		cfound;
-	bool		pred_op_negated;
-	Oid			pred_op_negator,
-				clause_op_negator,
-				test_op = InvalidOid;
-	Oid			opfamily_id;
+	Oid			test_op = InvalidOid;
 	bool		found = false;
-	StrategyNumber pred_strategy,
-				clause_strategy,
-				test_strategy;
-	Oid			clause_righttype;
-	CatCList   *catlist;
-	int			i;
+	List	   *pred_op_infos,
+			   *clause_op_infos;
+	ListCell   *lcp,
+			   *lcc;
 
 	/*
 	 * Find or make a cache entry for this pair of operators.
@@ -1629,135 +1624,71 @@ get_btree_test_op(Oid pred_op, Oid clause_op, bool refute_it)
 	 * corresponding test operator.  This should work for any logically
 	 * consistent opfamilies.
 	 */
-	catlist = SearchSysCacheList1(AMOPOPID, ObjectIdGetDatum(pred_op));
+	clause_op_infos = get_op_btree_interpretation(clause_op);
+	if (clause_op_infos)
+		pred_op_infos = get_op_btree_interpretation(pred_op);
+	else							/* no point in looking */
+		pred_op_infos = NIL;
 
-	/*
-	 * If we couldn't find any opfamily containing the pred_op, perhaps it is
-	 * a <> operator.  See if it has a negator that is in an opfamily.
-	 */
-	pred_op_negated = false;
-	if (catlist->n_members == 0)
+	foreach(lcp, pred_op_infos)
 	{
-		pred_op_negator = get_negator(pred_op);
-		if (OidIsValid(pred_op_negator))
+		OpBtreeInterpretation *pred_op_info = lfirst(lcp);
+		Oid			opfamily_id = pred_op_info->opfamily_id;
+
+		foreach(lcc, clause_op_infos)
 		{
-			pred_op_negated = true;
-			ReleaseSysCacheList(catlist);
-			catlist = SearchSysCacheList1(AMOPOPID,
-										  ObjectIdGetDatum(pred_op_negator));
-		}
-	}
+			OpBtreeInterpretation *clause_op_info = lfirst(lcc);
+			StrategyNumber pred_strategy,
+						clause_strategy,
+						test_strategy;
 
-	/* Also may need the clause_op's negator */
-	clause_op_negator = get_negator(clause_op);
-
-	/* Now search the opfamilies */
-	for (i = 0; i < catlist->n_members; i++)
-	{
-		HeapTuple	pred_tuple = &catlist->members[i]->tuple;
-		Form_pg_amop pred_form = (Form_pg_amop) GETSTRUCT(pred_tuple);
-		HeapTuple	clause_tuple;
-
-		/* Must be btree */
-		if (pred_form->amopmethod != BTREE_AM_OID)
-			continue;
-
-		/* Get the predicate operator's btree strategy number */
-		opfamily_id = pred_form->amopfamily;
-		pred_strategy = (StrategyNumber) pred_form->amopstrategy;
-		Assert(pred_strategy >= 1 && pred_strategy <= 5);
-
-		if (pred_op_negated)
-		{
-			/* Only consider negators that are = */
-			if (pred_strategy != BTEqualStrategyNumber)
+			/* Must find them in same opfamily */
+			if (opfamily_id != clause_op_info->opfamily_id)
 				continue;
-			pred_strategy = BTNE;
-		}
+			/* Lefttypes should match */
+			Assert(clause_op_info->oplefttype == pred_op_info->oplefttype);
 
-		/*
-		 * From the same opfamily, find a strategy number for the clause_op,
-		 * if possible
-		 */
-		clause_tuple = SearchSysCache3(AMOPOPID,
-									   ObjectIdGetDatum(clause_op),
-									   CharGetDatum(AMOP_SEARCH),
-									   ObjectIdGetDatum(opfamily_id));
-		if (HeapTupleIsValid(clause_tuple))
-		{
-			Form_pg_amop clause_form = (Form_pg_amop) GETSTRUCT(clause_tuple);
+			pred_strategy = pred_op_info->strategy;
+			clause_strategy = clause_op_info->strategy;
 
-			/* Get the restriction clause operator's strategy/datatype */
-			clause_strategy = (StrategyNumber) clause_form->amopstrategy;
-			Assert(clause_strategy >= 1 && clause_strategy <= 5);
-			Assert(clause_form->amoplefttype == pred_form->amoplefttype);
-			clause_righttype = clause_form->amoprighttype;
-			ReleaseSysCache(clause_tuple);
-		}
-		else if (OidIsValid(clause_op_negator))
-		{
-			clause_tuple = SearchSysCache3(AMOPOPID,
-										 ObjectIdGetDatum(clause_op_negator),
-										   CharGetDatum(AMOP_SEARCH),
-										   ObjectIdGetDatum(opfamily_id));
-			if (HeapTupleIsValid(clause_tuple))
+			/*
+			 * Look up the "test" strategy number in the implication table
+			 */
+			if (refute_it)
+				test_strategy = BT_refute_table[clause_strategy - 1][pred_strategy - 1];
+			else
+				test_strategy = BT_implic_table[clause_strategy - 1][pred_strategy - 1];
+
+			if (test_strategy == 0)
 			{
-				Form_pg_amop clause_form = (Form_pg_amop) GETSTRUCT(clause_tuple);
+				/* Can't determine implication using this interpretation */
+				continue;
+			}
 
-				/* Get the restriction clause operator's strategy/datatype */
-				clause_strategy = (StrategyNumber) clause_form->amopstrategy;
-				Assert(clause_strategy >= 1 && clause_strategy <= 5);
-				Assert(clause_form->amoplefttype == pred_form->amoplefttype);
-				clause_righttype = clause_form->amoprighttype;
-				ReleaseSysCache(clause_tuple);
-
-				/* Only consider negators that are = */
-				if (clause_strategy != BTEqualStrategyNumber)
-					continue;
-				clause_strategy = BTNE;
+			/*
+			 * See if opfamily has an operator for the test strategy and the
+			 * datatypes.
+			 */
+			if (test_strategy == BTNE)
+			{
+				test_op = get_opfamily_member(opfamily_id,
+											  pred_op_info->oprighttype,
+											  clause_op_info->oprighttype,
+											  BTEqualStrategyNumber);
+				if (OidIsValid(test_op))
+					test_op = get_negator(test_op);
 			}
 			else
+			{
+				test_op = get_opfamily_member(opfamily_id,
+											  pred_op_info->oprighttype,
+											  clause_op_info->oprighttype,
+											  test_strategy);
+			}
+
+			if (!OidIsValid(test_op))
 				continue;
-		}
-		else
-			continue;
 
-		/*
-		 * Look up the "test" strategy number in the implication table
-		 */
-		if (refute_it)
-			test_strategy = BT_refute_table[clause_strategy - 1][pred_strategy - 1];
-		else
-			test_strategy = BT_implic_table[clause_strategy - 1][pred_strategy - 1];
-
-		if (test_strategy == 0)
-		{
-			/* Can't determine implication using this interpretation */
-			continue;
-		}
-
-		/*
-		 * See if opfamily has an operator for the test strategy and the
-		 * datatypes.
-		 */
-		if (test_strategy == BTNE)
-		{
-			test_op = get_opfamily_member(opfamily_id,
-										  pred_form->amoprighttype,
-										  clause_righttype,
-										  BTEqualStrategyNumber);
-			if (OidIsValid(test_op))
-				test_op = get_negator(test_op);
-		}
-		else
-		{
-			test_op = get_opfamily_member(opfamily_id,
-										  pred_form->amoprighttype,
-										  clause_righttype,
-										  test_strategy);
-		}
-		if (OidIsValid(test_op))
-		{
 			/*
 			 * Last check: test_op must be immutable.
 			 *
@@ -1773,9 +1704,13 @@ get_btree_test_op(Oid pred_op, Oid clause_op, bool refute_it)
 				break;
 			}
 		}
+
+		if (found)
+			break;
 	}
 
-	ReleaseSysCacheList(catlist);
+	list_free_deep(pred_op_infos);
+	list_free_deep(clause_op_infos);
 
 	if (!found)
 	{

src/backend/parser/parse_expr.c

@@ -2170,8 +2170,7 @@ make_row_comparison_op(ParseState *pstate, List *opname,
 	List	   *opfamilies;
 	ListCell   *l,
 			   *r;
-	List	  **opfamily_lists;
-	List	  **opstrat_lists;
+	List	  **opinfo_lists;
 	Bitmapset  *strats;
 	int			nopers;
 	int			i;
@@ -2241,8 +2240,7 @@ make_row_comparison_op(ParseState *pstate, List *opname,
 	 * containing the operators, and see which interpretations (strategy
 	 * numbers) exist for each operator.
 	 */
-	opfamily_lists = (List **) palloc(nopers * sizeof(List *));
-	opstrat_lists = (List **) palloc(nopers * sizeof(List *));
+	opinfo_lists = (List **) palloc(nopers * sizeof(List *));
 	strats = NULL;
 	i = 0;
 	foreach(l, opexprs)
@@ -2251,17 +2249,18 @@ make_row_comparison_op(ParseState *pstate, List *opname,
 		Bitmapset  *this_strats;
 		ListCell   *j;
 
-		get_op_btree_interpretation(opno,
-									&opfamily_lists[i], &opstrat_lists[i]);
+		opinfo_lists[i] = get_op_btree_interpretation(opno);
 
 		/*
-		 * convert strategy number list to a Bitmapset to make the
+		 * convert strategy numbers into a Bitmapset to make the
 		 * intersection calculation easy.
 		 */
 		this_strats = NULL;
-		foreach(j, opstrat_lists[i])
+		foreach(j, opinfo_lists[i])
 		{
-			this_strats = bms_add_member(this_strats, lfirst_int(j));
+			OpBtreeInterpretation *opinfo = lfirst(j);
+
+			this_strats = bms_add_member(this_strats, opinfo->strategy);
 		}
 		if (i == 0)
 			strats = this_strats;
@@ -2309,14 +2308,15 @@ make_row_comparison_op(ParseState *pstate, List *opname,
 	for (i = 0; i < nopers; i++)
 	{
 		Oid			opfamily = InvalidOid;
+		ListCell   *j;
 
-		forboth(l, opfamily_lists[i], r, opstrat_lists[i])
+		foreach(j, opinfo_lists[i])
 		{
-			int			opstrat = lfirst_int(r);
+			OpBtreeInterpretation *opinfo = lfirst(j);
 
-			if (opstrat == rctype)
+			if (opinfo->strategy == rctype)
 			{
-				opfamily = lfirst_oid(l);
+				opfamily = opinfo->opfamily_id;
 				break;
 			}
 		}

src/backend/utils/cache/lsyscache.c

@@ -623,52 +623,30 @@ get_op_hash_functions(Oid opno,
 /*
  * get_op_btree_interpretation
  *		Given an operator's OID, find out which btree opfamilies it belongs to,
- *		and what strategy number it has within each one.  The results are
- *		returned as an OID list and a parallel integer list.
+ *		and what properties it has within each one.  The results are returned
+ *		as a palloc'd list of OpBtreeInterpretation structs.
  *
  * In addition to the normal btree operators, we consider a <> operator to be
  * a "member" of an opfamily if its negator is an equality operator of the
  * opfamily.  ROWCOMPARE_NE is returned as the strategy number for this case.
  */
-void
-get_op_btree_interpretation(Oid opno, List **opfamilies, List **opstrats)
+List *
+get_op_btree_interpretation(Oid opno)
 {
+	List	   *result = NIL;
+	OpBtreeInterpretation *thisresult;
 	CatCList   *catlist;
-	bool		op_negated;
 	int			i;
 
-	*opfamilies = NIL;
-	*opstrats = NIL;
-
 	/*
 	 * Find all the pg_amop entries containing the operator.
 	 */
 	catlist = SearchSysCacheList1(AMOPOPID, ObjectIdGetDatum(opno));
 
-	/*
-	 * If we can't find any opfamily containing the op, perhaps it is a <>
-	 * operator.  See if it has a negator that is in an opfamily.
-	 */
-	op_negated = false;
-	if (catlist->n_members == 0)
-	{
-		Oid			op_negator = get_negator(opno);
-
-		if (OidIsValid(op_negator))
-		{
-			op_negated = true;
-			ReleaseSysCacheList(catlist);
-			catlist = SearchSysCacheList1(AMOPOPID,
-										  ObjectIdGetDatum(op_negator));
-		}
-	}
-
-	/* Now search the opfamilies */
 	for (i = 0; i < catlist->n_members; i++)
 	{
 		HeapTuple	op_tuple = &catlist->members[i]->tuple;
 		Form_pg_amop op_form = (Form_pg_amop) GETSTRUCT(op_tuple);
-		Oid			opfamily_id;
 		StrategyNumber op_strategy;
 
 		/* must be btree */
@@ -676,23 +654,66 @@ get_op_btree_interpretation(Oid opno, List **opfamilies, List **opstrats)
 			continue;
 
 		/* Get the operator's btree strategy number */
-		opfamily_id = op_form->amopfamily;
 		op_strategy = (StrategyNumber) op_form->amopstrategy;
 		Assert(op_strategy >= 1 && op_strategy <= 5);
 
-		if (op_negated)
-		{
-			/* Only consider negators that are = */
-			if (op_strategy != BTEqualStrategyNumber)
-				continue;
-			op_strategy = ROWCOMPARE_NE;
-		}
-
-		*opfamilies = lappend_oid(*opfamilies, opfamily_id);
-		*opstrats = lappend_int(*opstrats, op_strategy);
+		thisresult = (OpBtreeInterpretation *)
+			palloc(sizeof(OpBtreeInterpretation));
+		thisresult->opfamily_id = op_form->amopfamily;
+		thisresult->strategy = op_strategy;
+		thisresult->oplefttype = op_form->amoplefttype;
+		thisresult->oprighttype = op_form->amoprighttype;
+		result = lappend(result, thisresult);
 	}
 
 	ReleaseSysCacheList(catlist);
+
+	/*
+	 * If we didn't find any btree opfamily containing the operator, perhaps
+	 * it is a <> operator.  See if it has a negator that is in an opfamily.
+	 */
+	if (result == NIL)
+	{
+		Oid			op_negator = get_negator(opno);
+
+		if (OidIsValid(op_negator))
+		{
+			catlist = SearchSysCacheList1(AMOPOPID,
+										  ObjectIdGetDatum(op_negator));
+
+			for (i = 0; i < catlist->n_members; i++)
+			{
+				HeapTuple	op_tuple = &catlist->members[i]->tuple;
+				Form_pg_amop op_form = (Form_pg_amop) GETSTRUCT(op_tuple);
+				StrategyNumber op_strategy;
+
+				/* must be btree */
+				if (op_form->amopmethod != BTREE_AM_OID)
+					continue;
+
+				/* Get the operator's btree strategy number */
+				op_strategy = (StrategyNumber) op_form->amopstrategy;
+				Assert(op_strategy >= 1 && op_strategy <= 5);
+
+				/* Only consider negators that are = */
+				if (op_strategy != BTEqualStrategyNumber)
+					continue;
+
+				/* OK, report it with "strategy" ROWCOMPARE_NE */
+				thisresult = (OpBtreeInterpretation *)
+					palloc(sizeof(OpBtreeInterpretation));
+				thisresult->opfamily_id = op_form->amopfamily;
+				thisresult->strategy = ROWCOMPARE_NE;
+				thisresult->oplefttype = op_form->amoplefttype;
+				thisresult->oprighttype = op_form->amoprighttype;
+				result = lappend(result, thisresult);
+			}
+
+			ReleaseSysCacheList(catlist);
+		}
+	}
+
+	return result;
 }
 
 /*

src/include/utils/lsyscache.h

@@ -17,6 +17,15 @@
 #include "access/htup.h"
 #include "nodes/pg_list.h"
 
+/* Result list element for get_op_btree_interpretation */
+typedef struct OpBtreeInterpretation
+{
+	Oid			opfamily_id;		/* btree opfamily containing operator */
+	int			strategy;			/* its strategy number */
+	Oid			oplefttype;			/* declared left input datatype */
+	Oid			oprighttype;		/* declared right input datatype */
+} OpBtreeInterpretation;
+
 /* I/O function selector for get_type_io_data */
 typedef enum IOFuncSelector
 {
@@ -50,8 +59,7 @@ extern bool get_compatible_hash_operators(Oid opno,
 							  Oid *lhs_opno, Oid *rhs_opno);
 extern bool get_op_hash_functions(Oid opno,
 					  RegProcedure *lhs_procno, RegProcedure *rhs_procno);
-extern void get_op_btree_interpretation(Oid opno,
-							List **opfamilies, List **opstrats);
+extern List *get_op_btree_interpretation(Oid opno);
 extern bool equality_ops_are_compatible(Oid opno1, Oid opno2);
 extern Oid get_opfamily_proc(Oid opfamily, Oid lefttype, Oid righttype,
 				  int16 procnum);