Fix failure to check whether a rowtype's component types are sortable.

The existence of a btree opclass accepting composite types caused us to
assume that every composite type is sortable.  This isn't true of course;
we need to check if the column types are all sortable.  There was logic
for this for the case of array comparison (ie, check that the element
type is sortable), but we missed the point for rowtypes.  Per Teodor's
report of an ANALYZE failure for an unsortable composite type.

Rather than just add some more ad-hoc logic for this, I moved knowledge of
the issue into typcache.c.  The typcache will now only report out array_eq,
record_cmp, and friends as usable operators if the array or composite type
will work with those functions.

Unfortunately we don't have enough info to do this for anonymous RECORD
types; in that case, just assume it will work, and take the runtime failure
as before if it doesn't.

This patch might be a candidate for back-patching at some point, but
given the lack of complaints from the field, I'd rather just test it in
HEAD for now.

Note: most of the places touched in this patch will need further work
when we get around to supporting hashing of record types.

src/backend/optimizer/plan/subselect.c

@@ -891,6 +891,7 @@ hash_ok_operator(OpExpr *expr)
 	if (opid == ARRAY_EQ_OP)
 	{
 		/* array_eq is strict, but must check input type to ensure hashable */
+		/* XXX record_eq will need same treatment when it becomes hashable */
 		Node	   *leftarg = linitial(expr->args);
 
 		return op_hashjoinable(opid, exprType(leftarg));

src/backend/parser/parse_oper.c

@@ -211,42 +211,6 @@ get_sort_group_operators(Oid argtype,
 	gt_opr = typentry->gt_opr;
 	hashable = OidIsValid(typentry->hash_proc);
 
-	/*
-	 * If the datatype is an array, then we can use array_lt and friends ...
-	 * but only if there are suitable operators for the element type.
-	 * Likewise, array types are only hashable if the element type is. Testing
-	 * all three operator IDs here should be redundant, but let's do it
-	 * anyway.
-	 */
-	if (lt_opr == ARRAY_LT_OP ||
-		eq_opr == ARRAY_EQ_OP ||
-		gt_opr == ARRAY_GT_OP)
-	{
-		Oid			elem_type = get_base_element_type(argtype);
-
-		if (OidIsValid(elem_type))
-		{
-			typentry = lookup_type_cache(elem_type, cache_flags);
-			if (!OidIsValid(typentry->eq_opr))
-			{
-				/* element type is neither sortable nor hashable */
-				lt_opr = eq_opr = gt_opr = InvalidOid;
-			}
-			else if (!OidIsValid(typentry->lt_opr) ||
-					 !OidIsValid(typentry->gt_opr))
-			{
-				/* element type is hashable but not sortable */
-				lt_opr = gt_opr = InvalidOid;
-			}
-			hashable = OidIsValid(typentry->hash_proc);
-		}
-		else
-		{
-			lt_opr = eq_opr = gt_opr = InvalidOid;		/* bogus array type? */
-			hashable = false;
-		}
-	}
-
 	/* Report errors if needed */
 	if ((needLT && !OidIsValid(lt_opr)) ||
 		(needGT && !OidIsValid(gt_opr)))

src/backend/utils/cache/lsyscache.c

@@ -33,6 +33,7 @@
 #include "utils/array.h"
 #include "utils/builtins.h"
 #include "utils/datum.h"
+#include "utils/fmgroids.h"
 #include "utils/lsyscache.h"
 #include "utils/syscache.h"
 #include "utils/typcache.h"
@@ -1120,34 +1121,35 @@ op_input_types(Oid opno, Oid *lefttype, Oid *righttype)
  * opfamily entries for this operator and associated sortops.  The pg_operator
  * flag is just a hint to tell the planner whether to bother looking.)
  *
- * In some cases (currently only array_eq), mergejoinability depends on the
- * specific input data type the operator is invoked for, so that must be
- * passed as well.	We currently assume that only one input's type is needed
- * to check this --- by convention, pass the left input's data type.
+ * In some cases (currently only array_eq and record_eq), mergejoinability
+ * depends on the specific input data type the operator is invoked for, so
+ * that must be passed as well. We currently assume that only one input's type
+ * is needed to check this --- by convention, pass the left input's data type.
  */
 bool
 op_mergejoinable(Oid opno, Oid inputtype)
 {
-	HeapTuple	tp;
 	bool		result = false;
+	HeapTuple	tp;
+	TypeCacheEntry *typentry;
 
-	if (opno == ARRAY_EQ_OP)
-	{
 	/*
-		 * For array_eq, can sort if element type has a default btree opclass.
-		 * We could use GetDefaultOpClass, but that's fairly expensive and not
-		 * cached, so let's use the typcache instead.
+	 * For array_eq or record_eq, we can sort if the element or field types
+	 * are all sortable.  We could implement all the checks for that here, but
+	 * the typcache already does that and caches the results too, so let's
+	 * rely on the typcache.
 	 */
-		Oid			elem_type = get_base_element_type(inputtype);
-
-		if (OidIsValid(elem_type))
+	if (opno == ARRAY_EQ_OP)
 	{
-			TypeCacheEntry *typentry;
-
-			typentry = lookup_type_cache(elem_type, TYPECACHE_BTREE_OPFAMILY);
-			if (OidIsValid(typentry->btree_opf))
+		typentry = lookup_type_cache(inputtype, TYPECACHE_CMP_PROC);
+		if (typentry->cmp_proc == F_BTARRAYCMP)
 			result = true;
 	}
+	else if (opno == RECORD_EQ_OP)
+	{
+		typentry = lookup_type_cache(inputtype, TYPECACHE_CMP_PROC);
+		if (typentry->cmp_proc == F_BTRECORDCMP)
+			result = true;
 	}
 	else
 	{
@@ -1178,23 +1180,18 @@ op_mergejoinable(Oid opno, Oid inputtype)
 bool
 op_hashjoinable(Oid opno, Oid inputtype)
 {
-	HeapTuple	tp;
 	bool		result = false;
+	HeapTuple	tp;
+	TypeCacheEntry *typentry;
 
+	/* As in op_mergejoinable, let the typcache handle the hard cases */
+	/* Eventually we'll need a similar case for record_eq ... */
 	if (opno == ARRAY_EQ_OP)
 	{
-		/* For array_eq, can hash if element type has a default hash opclass */
-		Oid			elem_type = get_base_element_type(inputtype);
-
-		if (OidIsValid(elem_type))
-		{
-			TypeCacheEntry *typentry;
-
-			typentry = lookup_type_cache(elem_type, TYPECACHE_HASH_OPFAMILY);
-			if (OidIsValid(typentry->hash_opf))
+		typentry = lookup_type_cache(inputtype, TYPECACHE_HASH_PROC);
+		if (typentry->hash_proc == F_HASH_ARRAY)
 			result = true;
 	}
-	}
 	else
 	{
 		/* For all other operators, rely on pg_operator.oprcanhash */

src/include/catalog/pg_operator.h

@@ -1647,12 +1647,15 @@ DESCR("text search match");
 /* generic record comparison operators */
 DATA(insert OID = 2988 (  "="	   PGNSP PGUID b t f 2249 2249 16 2988 2989 record_eq eqsel eqjoinsel ));
 DESCR("equal");
+#define RECORD_EQ_OP 2988
 DATA(insert OID = 2989 (  "<>"	   PGNSP PGUID b f f 2249 2249 16 2989 2988 record_ne neqsel neqjoinsel ));
 DESCR("not equal");
 DATA(insert OID = 2990 (  "<"	   PGNSP PGUID b f f 2249 2249 16 2991 2993 record_lt scalarltsel scalarltjoinsel ));
 DESCR("less than");
+#define RECORD_LT_OP 2990
 DATA(insert OID = 2991 (  ">"	   PGNSP PGUID b f f 2249 2249 16 2990 2992 record_gt scalargtsel scalargtjoinsel ));
 DESCR("greater than");
+#define RECORD_GT_OP 2991
 DATA(insert OID = 2992 (  "<="	   PGNSP PGUID b f f 2249 2249 16 2993 2991 record_le scalarltsel scalarltjoinsel ));
 DESCR("less than or equal");
 DATA(insert OID = 2993 (  ">="	   PGNSP PGUID b f f 2249 2249 16 2992 2990 record_ge scalargtsel scalargtjoinsel ));

src/include/utils/typcache.h

@@ -39,7 +39,9 @@ typedef struct TypeCacheEntry
 	 * Information obtained from opfamily entries
 	 *
 	 * These will be InvalidOid if no match could be found, or if the
-	 * information hasn't yet been requested.
+	 * information hasn't yet been requested.  Also note that for array and
+	 * composite types, typcache.c checks that the contained types are
+	 * comparable or hashable before allowing eq_opr etc to become set.
 	 */
 	Oid			btree_opf;		/* the default btree opclass' family */
 	Oid			btree_opintype; /* the default btree opclass' opcintype */
@@ -55,8 +57,8 @@ typedef struct TypeCacheEntry
 	 * Pre-set-up fmgr call info for the equality operator, the btree
 	 * comparison function, and the hash calculation function.	These are kept
 	 * in the type cache to avoid problems with memory leaks in repeated calls
-	 * to array_eq, array_cmp, hash_array.	There is not currently a need to
-	 * maintain call info for the lt_opr or gt_opr.
+	 * to functions such as array_eq, array_cmp, hash_array.  There is not
+	 * currently a need to maintain call info for the lt_opr or gt_opr.
 	 */
 	FmgrInfo	eq_opr_finfo;
 	FmgrInfo	cmp_proc_finfo;
@@ -69,6 +71,9 @@ typedef struct TypeCacheEntry
 	 */
 	TupleDesc	tupDesc;
 
+	/* Private data, for internal use of typcache.c only */
+	int			flags;			/* flags about what we've computed */
+
 	/*
 	 * Private information about an enum type.	NULL if not enum or
 	 * information hasn't been requested.

src/test/regress/expected/rowtypes.out

@@ -286,6 +286,16 @@ select row(1,1.1) = any (array[ row(7,7.7), row(1,1.0), row(0,0.0) ]);
  f
 (1 row)
 
+-- Check behavior with a non-comparable rowtype
+create type cantcompare as (p point, r float8);
+create temp table cc (f1 cantcompare);
+insert into cc values('("(1,2)",3)');
+insert into cc values('("(4,5)",6)');
+select * from cc order by f1; -- fail, but should complain about cantcompare
+ERROR:  could not identify an ordering operator for type cantcompare
+LINE 1: select * from cc order by f1;
+                                  ^
+HINT:  Use an explicit ordering operator or modify the query.
 --
 -- Test case derived from bug #5716: check multiple uses of a rowtype result
 --

src/test/regress/sql/rowtypes.sql

@@ -118,6 +118,13 @@ select array[ row(1,2), row(3,4), row(5,6) ];
 select row(1,1.1) = any (array[ row(7,7.7), row(1,1.1), row(0,0.0) ]);
 select row(1,1.1) = any (array[ row(7,7.7), row(1,1.0), row(0,0.0) ]);
 
+-- Check behavior with a non-comparable rowtype
+create type cantcompare as (p point, r float8);
+create temp table cc (f1 cantcompare);
+insert into cc values('("(1,2)",3)');
+insert into cc values('("(4,5)",6)');
+select * from cc order by f1; -- fail, but should complain about cantcompare
+
 --
 -- Test case derived from bug #5716: check multiple uses of a rowtype result
 --