More code review for rangetypes patch.

Fix up some infelicitous coding in DefineRange, and add some missing error
checks.  Rearrange operator strategy number assignments for GiST anyrange
opclass so that they don't make such a mess of opr_sanity's table of
operator names associated with different strategy numbers.  Assign
hopefully-temporary selectivity estimators to range operators that didn't
have one --- poor as the estimates are, they're still a lot better than the
default 0.5 estimate, and they'll shut up the opr_sanity test that wants to
see selectivity estimators on all built-in operators.

src/backend/utils/adt/rangetypes_gist.c

@@ -21,18 +21,19 @@
 
 
 /* Operator strategy numbers used in the GiST range opclass */
-#define RANGESTRAT_EQ					1
-#define RANGESTRAT_NE					2
+/* Numbers are chosen to match up operator names with existing usages */
+#define RANGESTRAT_BEFORE				1
+#define RANGESTRAT_OVERLEFT				2
 #define RANGESTRAT_OVERLAPS				3
-#define RANGESTRAT_CONTAINS_ELEM		4
-#define RANGESTRAT_ELEM_CONTAINED_BY	5
-#define RANGESTRAT_CONTAINS				6
-#define RANGESTRAT_CONTAINED_BY			7
-#define RANGESTRAT_BEFORE				8
-#define RANGESTRAT_AFTER				9
-#define RANGESTRAT_OVERLEFT				10
-#define RANGESTRAT_OVERRIGHT			11
-#define RANGESTRAT_ADJACENT				12
+#define RANGESTRAT_OVERRIGHT			4
+#define RANGESTRAT_AFTER				5
+#define RANGESTRAT_ADJACENT				6
+#define RANGESTRAT_CONTAINS				7
+#define RANGESTRAT_CONTAINED_BY			8
+#define RANGESTRAT_CONTAINS_ELEM		16
+#define RANGESTRAT_ELEM_CONTAINED_BY	17
+#define RANGESTRAT_EQ					18
+#define RANGESTRAT_NE					19
 
 #define RangeIsEmpty(r)  (range_get_flags(r) & RANGE_EMPTY)
 
@@ -460,47 +461,33 @@ range_gist_consistent_int(FmgrInfo *flinfo, StrategyNumber strategy,
 
 	switch (strategy)
 	{
-		case RANGESTRAT_EQ:
-			proc = range_contains;
-			break;
-		case RANGESTRAT_NE:
-			return true;
-			break;
-		case RANGESTRAT_OVERLAPS:
-			proc = range_overlaps;
-			break;
-		case RANGESTRAT_CONTAINS_ELEM:
-		case RANGESTRAT_CONTAINS:
-			proc = range_contains;
-			break;
-		case RANGESTRAT_ELEM_CONTAINED_BY:
-		case RANGESTRAT_CONTAINED_BY:
-			return true;
-			break;
 		case RANGESTRAT_BEFORE:
 			if (RangeIsEmpty(key))
 				return false;
 			proc = range_overright;
 			negate = true;
 			break;
-		case RANGESTRAT_AFTER:
-			if (RangeIsEmpty(key))
-				return false;
-			proc = range_overleft;
-			negate = true;
-			break;
 		case RANGESTRAT_OVERLEFT:
 			if (RangeIsEmpty(key))
 				return false;
 			proc = range_after;
 			negate = true;
 			break;
+		case RANGESTRAT_OVERLAPS:
+			proc = range_overlaps;
+			break;
 		case RANGESTRAT_OVERRIGHT:
 			if (RangeIsEmpty(key))
 				return false;
 			proc = range_before;
 			negate = true;
 			break;
+		case RANGESTRAT_AFTER:
+			if (RangeIsEmpty(key))
+				return false;
+			proc = range_overleft;
+			negate = true;
+			break;
 		case RANGESTRAT_ADJACENT:
 			if (RangeIsEmpty(key) || RangeIsEmpty(query))
 				return false;
@@ -510,6 +497,20 @@ range_gist_consistent_int(FmgrInfo *flinfo, StrategyNumber strategy,
 				return true;
 			proc = range_overlaps;
 			break;
+		case RANGESTRAT_CONTAINS:
+		case RANGESTRAT_CONTAINS_ELEM:
+			proc = range_contains;
+			break;
+		case RANGESTRAT_CONTAINED_BY:
+		case RANGESTRAT_ELEM_CONTAINED_BY:
+			return true;
+			break;
+		case RANGESTRAT_EQ:
+			proc = range_contains;
+			break;
+		case RANGESTRAT_NE:
+			return true;
+			break;
 		default:
 			elog(ERROR, "unrecognized range strategy: %d", strategy);
 			proc = NULL;		/* keep compiler quiet */
@@ -536,48 +537,48 @@ range_gist_consistent_leaf(FmgrInfo *flinfo, StrategyNumber strategy,
 
 	switch (strategy)
 	{
-		case RANGESTRAT_EQ:
-			proc = range_eq;
-			break;
-		case RANGESTRAT_NE:
-			proc = range_ne;
-			break;
-		case RANGESTRAT_OVERLAPS:
-			proc = range_overlaps;
-			break;
-		case RANGESTRAT_CONTAINS_ELEM:
-		case RANGESTRAT_CONTAINS:
-			proc = range_contains;
-			break;
-		case RANGESTRAT_ELEM_CONTAINED_BY:
-		case RANGESTRAT_CONTAINED_BY:
-			proc = range_contained_by;
-			break;
 		case RANGESTRAT_BEFORE:
 			if (RangeIsEmpty(key) || RangeIsEmpty(query))
 				return false;
 			proc = range_before;
 			break;
-		case RANGESTRAT_AFTER:
-			if (RangeIsEmpty(key) || RangeIsEmpty(query))
-				return false;
-			proc = range_after;
-			break;
 		case RANGESTRAT_OVERLEFT:
 			if (RangeIsEmpty(key) || RangeIsEmpty(query))
 				return false;
 			proc = range_overleft;
 			break;
+		case RANGESTRAT_OVERLAPS:
+			proc = range_overlaps;
+			break;
 		case RANGESTRAT_OVERRIGHT:
 			if (RangeIsEmpty(key) || RangeIsEmpty(query))
 				return false;
 			proc = range_overright;
 			break;
+		case RANGESTRAT_AFTER:
+			if (RangeIsEmpty(key) || RangeIsEmpty(query))
+				return false;
+			proc = range_after;
+			break;
 		case RANGESTRAT_ADJACENT:
 			if (RangeIsEmpty(key) || RangeIsEmpty(query))
 				return false;
 			proc = range_adjacent;
 			break;
+		case RANGESTRAT_CONTAINS:
+		case RANGESTRAT_CONTAINS_ELEM:
+			proc = range_contains;
+			break;
+		case RANGESTRAT_CONTAINED_BY:
+		case RANGESTRAT_ELEM_CONTAINED_BY:
+			proc = range_contained_by;
+			break;
+		case RANGESTRAT_EQ:
+			proc = range_eq;
+			break;
+		case RANGESTRAT_NE:
+			proc = range_ne;
+			break;
 		default:
 			elog(ERROR, "unrecognized range strategy: %d", strategy);
 			proc = NULL;		/* keep compiler quiet */

src/include/catalog/catversion.h

@@ -53,6 +53,6 @@
  */
 
 /*							yyyymmddN */
-#define CATALOG_VERSION_NO	201111171
+#define CATALOG_VERSION_NO	201111211
 
 #endif

src/include/catalog/pg_amop.h

@@ -726,17 +726,17 @@ DATA(insert (	3903   3831 3831 1 s	3882 405 0 ));
 /*
  * GiST range_ops
  */
-DATA(insert (	3919   3831 3831 1 s	3882 783 0 ));
-DATA(insert (	3919   3831 3831 2 s	3883 783 0 ));
+DATA(insert (	3919   3831 3831 1 s	3893 783 0 ));
+DATA(insert (	3919   3831 3831 2 s	3895 783 0 ));
 DATA(insert (	3919   3831 3831 3 s	3888 783 0 ));
-DATA(insert (	3919   3831 2283 4 s	3889 783 0 ));
-DATA(insert (	3919   2283 3831 5 s	3891 783 0 ));
-DATA(insert (	3919   3831 3831 6 s	3890 783 0 ));
-DATA(insert (	3919   3831 3831 7 s	3892 783 0 ));
-DATA(insert (	3919   3831 3831 8 s	3893 783 0 ));
-DATA(insert (	3919   3831 3831 9 s	3894 783 0 ));
-DATA(insert (	3919   3831 3831 10 s	3895 783 0 ));
-DATA(insert (	3919   3831 3831 11 s	3896 783 0 ));
-DATA(insert (	3919   3831 3831 12 s	3897 783 0 ));
+DATA(insert (	3919   3831 3831 4 s	3896 783 0 ));
+DATA(insert (	3919   3831 3831 5 s	3894 783 0 ));
+DATA(insert (	3919   3831 3831 6 s	3897 783 0 ));
+DATA(insert (	3919   3831 3831 7 s	3890 783 0 ));
+DATA(insert (	3919   3831 3831 8 s	3892 783 0 ));
+DATA(insert (	3919   3831 2283 16 s	3889 783 0 ));
+DATA(insert (	3919   2283 3831 17 s	3891 783 0 ));
+DATA(insert (	3919   3831 3831 18 s	3882 783 0 ));
+DATA(insert (	3919   3831 3831 19 s	3883 783 0 ));
 
 #endif   /* PG_AMOP_H */

src/include/catalog/pg_operator.h

@@ -1674,15 +1674,15 @@ DATA(insert OID = 3886 (  ">="	   PGNSP PGUID b f f 3831 3831 16 3885 3884 range
 DESCR("greater than or equal");
 DATA(insert OID = 3887 (  ">"	   PGNSP PGUID b f f 3831 3831 16 3884 3885 range_gt scalargtsel scalargtjoinsel ));
 DESCR("greater than");
-DATA(insert OID = 3888 (  "&&"	   PGNSP PGUID b f f 3831 3831 16 3888 0 range_overlaps - - ));
+DATA(insert OID = 3888 (  "&&"	   PGNSP PGUID b f f 3831 3831 16 3888 0 range_overlaps areasel areajoinsel ));
 DESCR("overlaps");
-DATA(insert OID = 3889 (  "@>"	   PGNSP PGUID b f f 3831 2283 16 3891 0 range_contains_elem - - ));
+DATA(insert OID = 3889 (  "@>"	   PGNSP PGUID b f f 3831 2283 16 3891 0 range_contains_elem contsel contjoinsel ));
 DESCR("contains");
-DATA(insert OID = 3890 (  "@>"	   PGNSP PGUID b f f 3831 3831 16 3892 0 range_contains - - ));
+DATA(insert OID = 3890 (  "@>"	   PGNSP PGUID b f f 3831 3831 16 3892 0 range_contains contsel contjoinsel ));
 DESCR("contains");
-DATA(insert OID = 3891 (  "<@"	   PGNSP PGUID b f f 2283 3831 16 3889 0 elem_contained_by_range - - ));
+DATA(insert OID = 3891 (  "<@"	   PGNSP PGUID b f f 2283 3831 16 3889 0 elem_contained_by_range contsel contjoinsel ));
 DESCR("is contained by");
-DATA(insert OID = 3892 (  "<@"	   PGNSP PGUID b f f 3831 3831 16 3890 0 range_contained_by - - ));
+DATA(insert OID = 3892 (  "<@"	   PGNSP PGUID b f f 3831 3831 16 3890 0 range_contained_by contsel contjoinsel ));
 DESCR("is contained by");
 DATA(insert OID = 3893 (  "<<"	   PGNSP PGUID b f f 3831 3831 16 3894 0 range_before scalarltsel scalarltjoinsel ));
 DESCR("is left of");
@@ -1692,7 +1692,7 @@ DATA(insert OID = 3895 (  "&<"	   PGNSP PGUID b f f 3831 3831 16 0 0 range_overl
 DESCR("overlaps or is left of");
 DATA(insert OID = 3896 (  "&>"	   PGNSP PGUID b f f 3831 3831 16 0 0 range_overright scalargtsel scalargtjoinsel ));
 DESCR("overlaps or is right of");
-DATA(insert OID = 3897 (  "-|-"	   PGNSP PGUID b f f 3831 3831 16 3897 0 range_adjacent - - ));
+DATA(insert OID = 3897 (  "-|-"	   PGNSP PGUID b f f 3831 3831 16 3897 0 range_adjacent contsel contjoinsel ));
 DESCR("is adjacent to");
 DATA(insert OID = 3898 (  "+"	   PGNSP PGUID b f f 3831 3831 3831 3898 0 range_union - - ));
 DESCR("range union");

src/test/regress/expected/opr_sanity.out

@@ -140,16 +140,16 @@ WHERE p1.oid < p2.oid AND
 -- need to be modified whenever new pairs of types are made binary-equivalent,
 -- or when new polymorphic built-in functions are added!
 -- Note: ignore aggregate functions here, since they all point to the same
--- dummy built-in function.
+-- dummy built-in function.  Likewise, ignore range constructor functions.
 SELECT DISTINCT p1.prorettype, p2.prorettype
 FROM pg_proc AS p1, pg_proc AS p2
 WHERE p1.oid != p2.oid AND
     p1.prosrc = p2.prosrc AND
     p1.prolang = 12 AND p2.prolang = 12 AND
     NOT p1.proisagg AND NOT p2.proisagg AND
-    (p1.prorettype < p2.prorettype) AND
-    -- range constructor functions are shared by all range types.
-    NOT p1.prosrc LIKE 'range_constructor%'
+    p1.prosrc NOT LIKE E'range\\_constructor_' AND
+    p2.prosrc NOT LIKE E'range\\_constructor_' AND
+    (p1.prorettype < p2.prorettype)
 ORDER BY 1, 2;
  prorettype | prorettype 
 ------------+------------
@@ -163,9 +163,9 @@ WHERE p1.oid != p2.oid AND
     p1.prosrc = p2.prosrc AND
     p1.prolang = 12 AND p2.prolang = 12 AND
     NOT p1.proisagg AND NOT p2.proisagg AND
-    (p1.proargtypes[0] < p2.proargtypes[0]) AND
-    -- range constructor functions are shared by all range types.
-    NOT p1.prosrc LIKE 'range_constructor%'
+    p1.prosrc NOT LIKE E'range\\_constructor_' AND
+    p2.prosrc NOT LIKE E'range\\_constructor_' AND
+    (p1.proargtypes[0] < p2.proargtypes[0])
 ORDER BY 1, 2;
  proargtypes | proargtypes 
 -------------+-------------
@@ -182,9 +182,9 @@ WHERE p1.oid != p2.oid AND
     p1.prosrc = p2.prosrc AND
     p1.prolang = 12 AND p2.prolang = 12 AND
     NOT p1.proisagg AND NOT p2.proisagg AND
-    (p1.proargtypes[1] < p2.proargtypes[1]) AND
-    -- range constructor functions are shared by all range types.
-    NOT p1.prosrc LIKE 'range_constructor%'
+    p1.prosrc NOT LIKE E'range\\_constructor_' AND
+    p2.prosrc NOT LIKE E'range\\_constructor_' AND
+    (p1.proargtypes[1] < p2.proargtypes[1])
 ORDER BY 1, 2;
  proargtypes | proargtypes 
 -------------+-------------
@@ -1021,34 +1021,28 @@ ORDER BY 1, 2, 3;
         403 |            5 | ~>~
         405 |            1 | =
         783 |            1 | <<
-        783 |            1 | =
         783 |            1 | @@
         783 |            2 | &<
-        783 |            2 | <>
         783 |            3 | &&
         783 |            4 | &>
-        783 |            4 | @>
-        783 |            5 | <@
         783 |            5 | >>
-        783 |            6 | @>
+        783 |            6 | -|-
         783 |            6 | ~=
-        783 |            7 | <@
         783 |            7 | @>
-        783 |            8 | <<
         783 |            8 | <@
         783 |            9 | &<|
-        783 |            9 | >>
-        783 |           10 | &<
         783 |           10 | <<|
         783 |           10 | <^
-        783 |           11 | &>
         783 |           11 | >^
         783 |           11 | |>>
-        783 |           12 | -|-
         783 |           12 | |&>
         783 |           13 | ~
         783 |           14 | @
         783 |           15 | <->
+        783 |           16 | @>
+        783 |           17 | <@
+        783 |           18 | =
+        783 |           19 | <>
         783 |           27 | @>
         783 |           28 | <@
         783 |           47 | @>
@@ -1061,7 +1055,7 @@ ORDER BY 1, 2, 3;
        2742 |            2 | @@@
        2742 |            3 | <@
        2742 |            4 | =
-(51 rows)
+(45 rows)
 
 -- Check that all opclass search operators have selectivity estimators.
 -- This is not absolutely required, but it seems a reasonable thing
@@ -1070,15 +1064,9 @@ SELECT p1.amopfamily, p1.amopopr, p2.oid, p2.oprname
 FROM pg_amop AS p1, pg_operator AS p2
 WHERE p1.amopopr = p2.oid AND p1.amoppurpose = 's' AND
     (p2.oprrest = 0 OR p2.oprjoin = 0);
- amopfamily | amopopr | oid  | oprname 
-------------+---------+------+---------
-       3919 |    3888 | 3888 | &&
-       3919 |    3889 | 3889 | @>
-       3919 |    3891 | 3891 | <@
-       3919 |    3890 | 3890 | @>
-       3919 |    3892 | 3892 | <@
-       3919 |    3897 | 3897 | -|-
-(6 rows)
+ amopfamily | amopopr | oid | oprname 
+------------+---------+-----+---------
+(0 rows)
 
 -- Check that each opclass in an opfamily has associated operators, that is
 -- ones whose oprleft matches opcintype (possibly by coercion).

src/test/regress/expected/plpgsql.out

@@ -4571,17 +4571,3 @@ ERROR:  value for domain orderedarray violates check constraint "sorted"
 CONTEXT:  PL/pgSQL function "testoa" line 5 at assignment
 drop function arrayassign1();
 drop function testoa(x1 int, x2 int, x3 int);
--- Test resolve_polymorphic_argtypes() codepath. It is only taken when
--- a function is invoked from a different backend from where it's defined,
--- so we create the a function with polymorphic argument, reconnect, and
--- and then call it.
-create function rangetypes_plpgsql(out a anyelement, b anyrange, c anyarray)
-  language plpgsql as
-  $$ begin a := upper(b) + c[1]; return; end; $$;
-\c -
-select rangetypes_plpgsql(int4range(1,10),ARRAY[2,20]);
- rangetypes_plpgsql 
---------------------
-                 12
-(1 row)
-

src/test/regress/expected/rangetypes.out

@@ -936,6 +936,20 @@ select range_add_bounds(numrange(1.0001, 123.123));
          124.1231
 (1 row)
 
+create function rangetypes_sql(q anyrange, b anyarray, out c anyelement)
+  as $$ select upper($1) + $2[1] $$
+  language sql;
+select rangetypes_sql(int4range(1,10), ARRAY[2,20]);
+ rangetypes_sql 
+----------------
+             12
+(1 row)
+
+select rangetypes_sql(numrange(1,10), ARRAY[2,20]);  -- match failure
+ERROR:  function rangetypes_sql(numrange, integer[]) does not exist
+LINE 1: select rangetypes_sql(numrange(1,10), ARRAY[2,20]);
+               ^
+HINT:  No function matches the given name and argument types. You might need to add explicit type casts.
 --
 -- Arrays of ranges
 --

src/test/regress/sql/opr_sanity.sql

@@ -125,7 +125,7 @@ WHERE p1.oid < p2.oid AND
 -- need to be modified whenever new pairs of types are made binary-equivalent,
 -- or when new polymorphic built-in functions are added!
 -- Note: ignore aggregate functions here, since they all point to the same
--- dummy built-in function.
+-- dummy built-in function.  Likewise, ignore range constructor functions.
 
 SELECT DISTINCT p1.prorettype, p2.prorettype
 FROM pg_proc AS p1, pg_proc AS p2
@@ -133,9 +133,9 @@ WHERE p1.oid != p2.oid AND
     p1.prosrc = p2.prosrc AND
     p1.prolang = 12 AND p2.prolang = 12 AND
     NOT p1.proisagg AND NOT p2.proisagg AND
-    (p1.prorettype < p2.prorettype) AND
-    -- range constructor functions are shared by all range types.
-    NOT p1.prosrc LIKE 'range_constructor%'
+    p1.prosrc NOT LIKE E'range\\_constructor_' AND
+    p2.prosrc NOT LIKE E'range\\_constructor_' AND
+    (p1.prorettype < p2.prorettype)
 ORDER BY 1, 2;
 
 SELECT DISTINCT p1.proargtypes[0], p2.proargtypes[0]
@@ -144,9 +144,9 @@ WHERE p1.oid != p2.oid AND
     p1.prosrc = p2.prosrc AND
     p1.prolang = 12 AND p2.prolang = 12 AND
     NOT p1.proisagg AND NOT p2.proisagg AND
-    (p1.proargtypes[0] < p2.proargtypes[0]) AND
-    -- range constructor functions are shared by all range types.
-    NOT p1.prosrc LIKE 'range_constructor%'
+    p1.prosrc NOT LIKE E'range\\_constructor_' AND
+    p2.prosrc NOT LIKE E'range\\_constructor_' AND
+    (p1.proargtypes[0] < p2.proargtypes[0])
 ORDER BY 1, 2;
 
 SELECT DISTINCT p1.proargtypes[1], p2.proargtypes[1]
@@ -155,9 +155,9 @@ WHERE p1.oid != p2.oid AND
     p1.prosrc = p2.prosrc AND
     p1.prolang = 12 AND p2.prolang = 12 AND
     NOT p1.proisagg AND NOT p2.proisagg AND
-    (p1.proargtypes[1] < p2.proargtypes[1]) AND
-    -- range constructor functions are shared by all range types.
-    NOT p1.prosrc LIKE 'range_constructor%'
+    p1.prosrc NOT LIKE E'range\\_constructor_' AND
+    p2.prosrc NOT LIKE E'range\\_constructor_' AND
+    (p1.proargtypes[1] < p2.proargtypes[1])
 ORDER BY 1, 2;
 
 SELECT DISTINCT p1.proargtypes[2], p2.proargtypes[2]

src/test/regress/sql/plpgsql.sql

@@ -3600,13 +3600,3 @@ select testoa(1,2,1); -- fail at update
 
 drop function arrayassign1();
 drop function testoa(x1 int, x2 int, x3 int);
-
--- Test resolve_polymorphic_argtypes() codepath. It is only taken when
--- a function is invoked from a different backend from where it's defined,
--- so we create the a function with polymorphic argument, reconnect, and
--- and then call it.
-create function rangetypes_plpgsql(out a anyelement, b anyrange, c anyarray)
-  language plpgsql as
-  $$ begin a := upper(b) + c[1]; return; end; $$;
-\c -
-select rangetypes_plpgsql(int4range(1,10),ARRAY[2,20]);

src/test/regress/sql/rangetypes.sql

@@ -327,6 +327,13 @@ create function range_add_bounds(anyrange)
 
 select range_add_bounds(numrange(1.0001, 123.123));
 
+create function rangetypes_sql(q anyrange, b anyarray, out c anyelement)
+  as $$ select upper($1) + $2[1] $$
+  language sql;
+
+select rangetypes_sql(int4range(1,10), ARRAY[2,20]);
+select rangetypes_sql(numrange(1,10), ARRAY[2,20]);  -- match failure
+
 --
 -- Arrays of ranges
 --