From 3d660d33aab2f1eb98367a84eb2addf3e0969c05 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 30 Jan 2015 12:30:38 -0500 Subject: [PATCH] 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 --- src/backend/utils/adt/rangetypes_selfuncs.c | 50 ++++++++++++++------- src/include/catalog/pg_operator.h | 4 +- src/test/regress/expected/rangetypes.out | 32 +++++++++++++ src/test/regress/sql/rangetypes.sql | 4 ++ 4 files changed, 72 insertions(+), 18 deletions(-) diff --git a/src/backend/utils/adt/rangetypes_selfuncs.c b/src/backend/utils/adt/rangetypes_selfuncs.c index 0499095315..a130b483f3 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: diff --git a/src/include/catalog/pg_operator.h b/src/include/catalog/pg_operator.h index d8a25f841d..af991d34f7 100644 --- 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 39db992729..35d0dd3f67 100644 --- 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 fad843a405..aa026cad0a 100644 --- 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,'[]'); -- 2.40.0