Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 3d660d3

Browse files
committed
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 parent 68fa75f commit 3d660d3

File tree

4 files changed

+72
-18
lines changed

4 files changed

+72
-18
lines changed

src/backend/utils/adt/rangetypes_selfuncs.c

+34-16
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ default_range_selectivity(Oid operator)
7373
return 0.005;
7474

7575
case OID_RANGE_CONTAINS_ELEM_OP:
76+
case OID_RANGE_ELEM_CONTAINED_OP:
7677

7778
/*
7879
* "range @> elem" is more or less identical to a scalar
@@ -86,6 +87,8 @@ default_range_selectivity(Oid operator)
8687
case OID_RANGE_GREATER_EQUAL_OP:
8788
case OID_RANGE_LEFT_OP:
8889
case OID_RANGE_RIGHT_OP:
90+
case OID_RANGE_OVERLAPS_LEFT_OP:
91+
case OID_RANGE_OVERLAPS_RIGHT_OP:
8992
/* these are similar to regular scalar inequalities */
9093
return DEFAULT_INEQ_SEL;
9194

@@ -109,7 +112,7 @@ rangesel(PG_FUNCTION_ARGS)
109112
Node *other;
110113
bool varonleft;
111114
Selectivity selec;
112-
TypeCacheEntry *typcache;
115+
TypeCacheEntry *typcache = NULL;
113116
RangeType *constrange = NULL;
114117

115118
/*
@@ -186,18 +189,27 @@ rangesel(PG_FUNCTION_ARGS)
186189
constrange = range_serialize(typcache, &lower, &upper, false);
187190
}
188191
}
189-
else
192+
else if (operator == OID_RANGE_ELEM_CONTAINED_OP)
193+
{
194+
/*
195+
* Here, the Var is the elem, not the range. For now we just punt and
196+
* return the default estimate. In future we could disassemble the
197+
* range constant and apply scalarineqsel ...
198+
*/
199+
}
200+
else if (((Const *) other)->consttype == vardata.vartype)
190201
{
191-
typcache = range_get_typcache(fcinfo, ((Const *) other)->consttype);
202+
/* Both sides are the same range type */
203+
typcache = range_get_typcache(fcinfo, vardata.vartype);
192204

193-
if (((Const *) other)->consttype == vardata.vartype)
194-
constrange = DatumGetRangeType(((Const *) other)->constvalue);
205+
constrange = DatumGetRangeType(((Const *) other)->constvalue);
195206
}
196207

197208
/*
198209
* If we got a valid constant on one side of the operator, proceed to
199210
* estimate using statistics. Otherwise punt and return a default constant
200-
* estimate.
211+
* estimate. Note that calc_rangesel need not handle
212+
* OID_RANGE_ELEM_CONTAINED_OP.
201213
*/
202214
if (constrange)
203215
selec = calc_rangesel(typcache, &vardata, constrange, operator);
@@ -270,31 +282,37 @@ calc_rangesel(TypeCacheEntry *typcache, VariableStatData *vardata,
270282
*/
271283
switch (operator)
272284
{
285+
/* these return false if either argument is empty */
273286
case OID_RANGE_OVERLAP_OP:
274287
case OID_RANGE_OVERLAPS_LEFT_OP:
275288
case OID_RANGE_OVERLAPS_RIGHT_OP:
276289
case OID_RANGE_LEFT_OP:
277290
case OID_RANGE_RIGHT_OP:
278-
/* these return false if either argument is empty */
291+
/* nothing is less than an empty range */
292+
case OID_RANGE_LESS_OP:
279293
selec = 0.0;
280294
break;
281295

296+
/* only empty ranges can be contained by an empty range */
282297
case OID_RANGE_CONTAINED_OP:
298+
/* only empty ranges are <= an empty range */
283299
case OID_RANGE_LESS_EQUAL_OP:
284-
case OID_RANGE_GREATER_EQUAL_OP:
285-
286-
/*
287-
* these return true when both args are empty, false if only
288-
* one is empty
289-
*/
290300
selec = empty_frac;
291301
break;
292302

293-
case OID_RANGE_CONTAINS_OP:
294303
/* everything contains an empty range */
304+
case OID_RANGE_CONTAINS_OP:
305+
/* everything is >= an empty range */
306+
case OID_RANGE_GREATER_EQUAL_OP:
295307
selec = 1.0;
296308
break;
297309

310+
/* all non-empty ranges are > an empty range */
311+
case OID_RANGE_GREATER_OP:
312+
selec = 1.0 - empty_frac;
313+
break;
314+
315+
/* an element cannot be empty */
298316
case OID_RANGE_CONTAINS_ELEM_OP:
299317
default:
300318
elog(ERROR, "unexpected operator %u", operator);
@@ -443,13 +461,13 @@ calc_hist_selectivity(TypeCacheEntry *typcache, VariableStatData *vardata,
443461
case OID_RANGE_GREATER_OP:
444462
hist_selec =
445463
1 - calc_hist_selectivity_scalar(typcache, &const_lower,
446-
hist_lower, nhist, true);
464+
hist_lower, nhist, false);
447465
break;
448466

449467
case OID_RANGE_GREATER_EQUAL_OP:
450468
hist_selec =
451469
1 - calc_hist_selectivity_scalar(typcache, &const_lower,
452-
hist_lower, nhist, false);
470+
hist_lower, nhist, true);
453471
break;
454472

455473
case OID_RANGE_LEFT_OP:

src/include/catalog/pg_operator.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -1723,10 +1723,10 @@ DESCR("less than or equal");
17231723
#define OID_RANGE_LESS_EQUAL_OP 3885
17241724
DATA(insert OID = 3886 ( ">=" PGNSP PGUID b f f 3831 3831 16 3885 3884 range_ge rangesel scalargtjoinsel ));
17251725
DESCR("greater than or equal");
1726-
#define OID_RANGE_GREATER_OP 3886
1726+
#define OID_RANGE_GREATER_EQUAL_OP 3886
17271727
DATA(insert OID = 3887 ( ">" PGNSP PGUID b f f 3831 3831 16 3884 3885 range_gt rangesel scalargtjoinsel ));
17281728
DESCR("greater than");
1729-
#define OID_RANGE_GREATER_EQUAL_OP 3887
1729+
#define OID_RANGE_GREATER_OP 3887
17301730
DATA(insert OID = 3888 ( "&&" PGNSP PGUID b f f 3831 3831 16 3888 0 range_overlaps rangesel areajoinsel ));
17311731
DESCR("overlaps");
17321732
#define OID_RANGE_OVERLAP_OP 3888

src/test/regress/expected/rangetypes.out

+32
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,11 @@ select * from numrange_test where nr = '[1.1, 2.2)';
260260
[1.1,2.2)
261261
(1 row)
262262

263+
select * from numrange_test where nr < 'empty';
264+
nr
265+
----
266+
(0 rows)
267+
263268
select * from numrange_test where nr < numrange(-1000.0, -1000.0,'[]');
264269
nr
265270
-------
@@ -287,6 +292,33 @@ select * from numrange_test where nr < numrange(1000.0, 1001.0,'[]');
287292
[1.7,1.7]
288293
(6 rows)
289294

295+
select * from numrange_test where nr <= 'empty';
296+
nr
297+
-------
298+
empty
299+
(1 row)
300+
301+
select * from numrange_test where nr >= 'empty';
302+
nr
303+
-----------
304+
(,)
305+
[3,)
306+
(,5)
307+
[1.1,2.2)
308+
empty
309+
[1.7,1.7]
310+
(6 rows)
311+
312+
select * from numrange_test where nr > 'empty';
313+
nr
314+
-----------
315+
(,)
316+
[3,)
317+
(,5)
318+
[1.1,2.2)
319+
[1.7,1.7]
320+
(5 rows)
321+
290322
select * from numrange_test where nr > numrange(-1001.0, -1000.0,'[]');
291323
nr
292324
-----------

src/test/regress/sql/rangetypes.sql

+4
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,13 @@ SELECT * FROM numrange_test WHERE 1.9 <@ nr;
6767
select * from numrange_test where nr = 'empty';
6868
select * from numrange_test where nr = '(1.1, 2.2)';
6969
select * from numrange_test where nr = '[1.1, 2.2)';
70+
select * from numrange_test where nr < 'empty';
7071
select * from numrange_test where nr < numrange(-1000.0, -1000.0,'[]');
7172
select * from numrange_test where nr < numrange(0.0, 1.0,'[]');
7273
select * from numrange_test where nr < numrange(1000.0, 1001.0,'[]');
74+
select * from numrange_test where nr <= 'empty';
75+
select * from numrange_test where nr >= 'empty';
76+
select * from numrange_test where nr > 'empty';
7377
select * from numrange_test where nr > numrange(-1001.0, -1000.0,'[]');
7478
select * from numrange_test where nr > numrange(0.0, 1.0,'[]');
7579
select * from numrange_test where nr > numrange(1000.0, 1000.0,'[]');

0 commit comments

Comments
 (0)