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

diff --git a/src/backend/utils/adt/rangetypes_selfuncs.c b/src/backend/utils/adt/rangetypes_selfuncs.c
index 0499095315f8e50a02b7e6ea7de8653f829e054e..a130b483f3c58ee465715b78dd1aea8d6151016e 100644 (file)
--- 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:

diff --git a/src/include/catalog/pg_operator.h b/src/include/catalog/pg_operator.h
index d8a25f841d239470cd45af358616e0d1db95c8b8..af991d34f729b92926d9dbbf2a55459fa9faba20 100644 (file)
--- a/src/include/catalog/pg_operator.h
+++ b/src/include/catalog/pg_operator.h
@@ -1723,10 +1723,10 @@ DESCR("less than or equal");
 #define OID_RANGE_LESS_EQUAL_OP 3885
 DATA(insert OID = 3886 (  ">="    PGNSP PGUID b f f 3831 3831 16 3885 3884 range_ge rangesel scalargtjoinsel ));
 DESCR("greater than or equal");
-#define OID_RANGE_GREATER_OP 3886
+#define OID_RANGE_GREATER_EQUAL_OP 3886
 DATA(insert OID = 3887 (  ">"     PGNSP PGUID b f f 3831 3831 16 3884 3885 range_gt rangesel scalargtjoinsel ));
 DESCR("greater than");
-#define OID_RANGE_GREATER_EQUAL_OP 3887
+#define OID_RANGE_GREATER_OP 3887
 DATA(insert OID = 3888 (  "&&"    PGNSP PGUID b f f 3831 3831 16 3888 0 range_overlaps rangesel areajoinsel ));
 DESCR("overlaps");
 #define OID_RANGE_OVERLAP_OP 3888

diff --git a/src/test/regress/expected/rangetypes.out b/src/test/regress/expected/rangetypes.out
index 39db9927291965f581461ef6c6e371c12bf1bbd8..35d0dd3f677e5c4d136997f7f867c83663576203 100644 (file)
--- a/src/test/regress/expected/rangetypes.out
+++ b/src/test/regress/expected/rangetypes.out
@@ -260,6 +260,11 @@ select * from numrange_test where nr = '[1.1, 2.2)';
  [1.1,2.2)
 (1 row)
 
+select * from numrange_test where nr < 'empty';
+ nr 
+----
+(0 rows)
+
 select * from numrange_test where nr < numrange(-1000.0, -1000.0,'[]');
   nr   
 -------
@@ -287,6 +292,33 @@ select * from numrange_test where nr < numrange(1000.0, 1001.0,'[]');
  [1.7,1.7]
 (6 rows)
 
+select * from numrange_test where nr <= 'empty';
+  nr   
+-------
+ empty
+(1 row)
+
+select * from numrange_test where nr >= 'empty';
+    nr     
+-----------
+ (,)
+ [3,)
+ (,5)
+ [1.1,2.2)
+ empty
+ [1.7,1.7]
+(6 rows)
+
+select * from numrange_test where nr > 'empty';
+    nr     
+-----------
+ (,)
+ [3,)
+ (,5)
+ [1.1,2.2)
+ [1.7,1.7]
+(5 rows)
+
 select * from numrange_test where nr > numrange(-1001.0, -1000.0,'[]');
     nr     
 -----------

diff --git a/src/test/regress/sql/rangetypes.sql b/src/test/regress/sql/rangetypes.sql
index fad843a405f599203b27e4a6d4cf9bc18a25e7b5..aa026cad0abc2898ac5203f117d82b13e5da8c98 100644 (file)
--- a/src/test/regress/sql/rangetypes.sql
+++ b/src/test/regress/sql/rangetypes.sql
@@ -67,9 +67,13 @@ SELECT * FROM numrange_test WHERE 1.9 <@ nr;
 select * from numrange_test where nr = 'empty';
 select * from numrange_test where nr = '(1.1, 2.2)';
 select * from numrange_test where nr = '[1.1, 2.2)';
+select * from numrange_test where nr < 'empty';
 select * from numrange_test where nr < numrange(-1000.0, -1000.0,'[]');
 select * from numrange_test where nr < numrange(0.0, 1.0,'[]');
 select * from numrange_test where nr < numrange(1000.0, 1001.0,'[]');
+select * from numrange_test where nr <= 'empty';
+select * from numrange_test where nr >= 'empty';
+select * from numrange_test where nr > 'empty';
 select * from numrange_test where nr > numrange(-1001.0, -1000.0,'[]');
 select * from numrange_test where nr > numrange(0.0, 1.0,'[]');
 select * from numrange_test where nr > numrange(1000.0, 1000.0,'[]');