Fix assorted oversights in range selectivity estimation.

calc_rangesel() failed outright when comparing range variables to empty
constant ranges with < or >=, as a result of missing cases in a switch.
It also produced a bogus estimate for > comparison to an empty range.

On top of that, the >= and > cases were mislabeled throughout.  For
nonempty constant ranges, they managed to produce the right answers
anyway as a result of counterbalancing typos.

Also, default_range_selectivity() omitted cases for elem <@ range,
range &< range, and range &> range, so that rather dubious defaults
were applied for these operators.

In passing, rearrange the code in rangesel() so that the elem <@ range
case is handled in a less opaque fashion.

Report and patch by Emre Hasegeli, some additional work by me

1 files changed, 34 insertions, 16 deletions

diff --git a/src/backend/utils/adt/rangetypes_selfuncs.c b/src/backend/utils/adt/rangetypes_selfuncs.c
index 71625740407..6862752c4b7 100644
--- a/src/backend/utils/adt/rangetypes_selfuncs.c
+++ b/src/backend/utils/adt/rangetypes_selfuncs.c
@@ -73,6 +73,7 @@ default_range_selectivity(Oid operator)
 			return 0.005;
 
 		case OID_RANGE_CONTAINS_ELEM_OP:
+		case OID_RANGE_ELEM_CONTAINED_OP:
 
 			/*
 			 * "range @> elem" is more or less identical to a scalar
@@ -86,6 +87,8 @@ default_range_selectivity(Oid operator)
 		case OID_RANGE_GREATER_EQUAL_OP:
 		case OID_RANGE_LEFT_OP:
 		case OID_RANGE_RIGHT_OP:
+		case OID_RANGE_OVERLAPS_LEFT_OP:
+		case OID_RANGE_OVERLAPS_RIGHT_OP:
 			/* these are similar to regular scalar inequalities */
 			return DEFAULT_INEQ_SEL;
 
@@ -109,7 +112,7 @@ rangesel(PG_FUNCTION_ARGS)
 	Node	   *other;
 	bool		varonleft;
 	Selectivity selec;
-	TypeCacheEntry *typcache;
+	TypeCacheEntry *typcache = NULL;
 	RangeType  *constrange = NULL;
 
 	/*
@@ -186,18 +189,27 @@ rangesel(PG_FUNCTION_ARGS)
 			constrange = range_serialize(typcache, &lower, &upper, false);
 		}
 	}
-	else
+	else if (operator == OID_RANGE_ELEM_CONTAINED_OP)
+	{
+		/*
+		 * Here, the Var is the elem, not the range.  For now we just punt and
+		 * return the default estimate.  In future we could disassemble the
+		 * range constant and apply scalarineqsel ...
+		 */
+	}
+	else if (((Const *) other)->consttype == vardata.vartype)
 	{
-		typcache = range_get_typcache(fcinfo, ((Const *) other)->consttype);
+		/* Both sides are the same range type */
+		typcache = range_get_typcache(fcinfo, vardata.vartype);
 
-		if (((Const *) other)->consttype == vardata.vartype)
-			constrange = DatumGetRangeType(((Const *) other)->constvalue);
+		constrange = DatumGetRangeType(((Const *) other)->constvalue);
 	}
 
 	/*
 	 * If we got a valid constant on one side of the operator, proceed to
 	 * estimate using statistics. Otherwise punt and return a default constant
-	 * estimate.
+	 * estimate.  Note that calc_rangesel need not handle
+	 * OID_RANGE_ELEM_CONTAINED_OP.
 	 */
 	if (constrange)
 		selec = calc_rangesel(typcache, &vardata, constrange, operator);
@@ -270,31 +282,37 @@ calc_rangesel(TypeCacheEntry *typcache, VariableStatData *vardata,
 		 */
 		switch (operator)
 		{
+				/* these return false if either argument is empty */
 			case OID_RANGE_OVERLAP_OP:
 			case OID_RANGE_OVERLAPS_LEFT_OP:
 			case OID_RANGE_OVERLAPS_RIGHT_OP:
 			case OID_RANGE_LEFT_OP:
 			case OID_RANGE_RIGHT_OP:
-				/* these return false if either argument is empty */
+				/* nothing is less than an empty range */
+			case OID_RANGE_LESS_OP:
 				selec = 0.0;
 				break;
 
+				/* only empty ranges can be contained by an empty range */
 			case OID_RANGE_CONTAINED_OP:
+				/* only empty ranges are <= an empty range */
 			case OID_RANGE_LESS_EQUAL_OP:
-			case OID_RANGE_GREATER_EQUAL_OP:
-
-				/*
-				 * these return true when both args are empty, false if only
-				 * one is empty
-				 */
 				selec = empty_frac;
 				break;
 
-			case OID_RANGE_CONTAINS_OP:
 				/* everything contains an empty range */
+			case OID_RANGE_CONTAINS_OP:
+				/* everything is >= an empty range */
+			case OID_RANGE_GREATER_EQUAL_OP:
 				selec = 1.0;
 				break;
 
+				/* all non-empty ranges are > an empty range */
+			case OID_RANGE_GREATER_OP:
+				selec = 1.0 - empty_frac;
+				break;
+
+				/* an element cannot be empty */
 			case OID_RANGE_CONTAINS_ELEM_OP:
 			default:
 				elog(ERROR, "unexpected operator %u", operator);
@@ -443,13 +461,13 @@ calc_hist_selectivity(TypeCacheEntry *typcache, VariableStatData *vardata,
 		case OID_RANGE_GREATER_OP:
 			hist_selec =
 				1 - calc_hist_selectivity_scalar(typcache, &const_lower,
-												 hist_lower, nhist, true);
+												 hist_lower, nhist, false);
 			break;
 
 		case OID_RANGE_GREATER_EQUAL_OP:
 			hist_selec =
 				1 - calc_hist_selectivity_scalar(typcache, &const_lower,
-												 hist_lower, nhist, false);
+												 hist_lower, nhist, true);
 			break;
 
 		case OID_RANGE_LEFT_OP: