Skip to content

Commit a4ffcc8

Browse files
tglsfdctglsfdc
committed
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.
1 parent 9b97b7f commit a4ffcc8

File tree

11 files changed

+342
-317
lines changed

11 files changed

+342
-317
lines changed

‎src/backend/commands/typecmds.c

Lines changed: 217 additions & 178 deletions
Large diffs are not rendered by default.

‎src/backend/utils/adt/rangetypes_gist.c

Lines changed: 57 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,19 @@
2121

2222

2323
/* Operator strategy numbers used in the GiST range opclass */
24-
#define RANGESTRAT_EQ 1
25-
#define RANGESTRAT_NE 2
24+
/* Numbers are chosen to match up operator names with existing usages */
25+
#define RANGESTRAT_BEFORE 1
26+
#define RANGESTRAT_OVERLEFT 2
2627
#define RANGESTRAT_OVERLAPS 3
27-
#define RANGESTRAT_CONTAINS_ELEM 4
28-
#define RANGESTRAT_ELEM_CONTAINED_BY 5
29-
#define RANGESTRAT_CONTAINS 6
30-
#define RANGESTRAT_CONTAINED_BY 7
31-
#define RANGESTRAT_BEFORE 8
32-
#define RANGESTRAT_AFTER 9
33-
#define RANGESTRAT_OVERLEFT 10
34-
#define RANGESTRAT_OVERRIGHT 11
35-
#define RANGESTRAT_ADJACENT 12
28+
#define RANGESTRAT_OVERRIGHT 4
29+
#define RANGESTRAT_AFTER 5
30+
#define RANGESTRAT_ADJACENT 6
31+
#define RANGESTRAT_CONTAINS 7
32+
#define RANGESTRAT_CONTAINED_BY 8
33+
#define RANGESTRAT_CONTAINS_ELEM 16
34+
#define RANGESTRAT_ELEM_CONTAINED_BY 17
35+
#define RANGESTRAT_EQ 18
36+
#define RANGESTRAT_NE 19
3637

3738
#define RangeIsEmpty(r) (range_get_flags(r) & RANGE_EMPTY)
3839

@@ -460,47 +461,33 @@ range_gist_consistent_int(FmgrInfo *flinfo, StrategyNumber strategy,
460461

461462
switch (strategy)
462463
{
463-
case RANGESTRAT_EQ:
464-
proc = range_contains;
465-
break;
466-
case RANGESTRAT_NE:
467-
return true;
468-
break;
469-
case RANGESTRAT_OVERLAPS:
470-
proc = range_overlaps;
471-
break;
472-
case RANGESTRAT_CONTAINS_ELEM:
473-
case RANGESTRAT_CONTAINS:
474-
proc = range_contains;
475-
break;
476-
case RANGESTRAT_ELEM_CONTAINED_BY:
477-
case RANGESTRAT_CONTAINED_BY:
478-
return true;
479-
break;
480464
case RANGESTRAT_BEFORE:
481465
if (RangeIsEmpty(key))
482466
return false;
483467
proc = range_overright;
484468
negate = true;
485469
break;
486-
case RANGESTRAT_AFTER:
487-
if (RangeIsEmpty(key))
488-
return false;
489-
proc = range_overleft;
490-
negate = true;
491-
break;
492470
case RANGESTRAT_OVERLEFT:
493471
if (RangeIsEmpty(key))
494472
return false;
495473
proc = range_after;
496474
negate = true;
497475
break;
476+
case RANGESTRAT_OVERLAPS:
477+
proc = range_overlaps;
478+
break;
498479
case RANGESTRAT_OVERRIGHT:
499480
if (RangeIsEmpty(key))
500481
return false;
501482
proc = range_before;
502483
negate = true;
503484
break;
485+
case RANGESTRAT_AFTER:
486+
if (RangeIsEmpty(key))
487+
return false;
488+
proc = range_overleft;
489+
negate = true;
490+
break;
504491
case RANGESTRAT_ADJACENT:
505492
if (RangeIsEmpty(key) || RangeIsEmpty(query))
506493
return false;
@@ -510,6 +497,20 @@ range_gist_consistent_int(FmgrInfo *flinfo, StrategyNumber strategy,
510497
return true;
511498
proc = range_overlaps;
512499
break;
500+
case RANGESTRAT_CONTAINS:
501+
case RANGESTRAT_CONTAINS_ELEM:
502+
proc = range_contains;
503+
break;
504+
case RANGESTRAT_CONTAINED_BY:
505+
case RANGESTRAT_ELEM_CONTAINED_BY:
506+
return true;
507+
break;
508+
case RANGESTRAT_EQ:
509+
proc = range_contains;
510+
break;
511+
case RANGESTRAT_NE:
512+
return true;
513+
break;
513514
default:
514515
elog(ERROR, "unrecognized range strategy: %d", strategy);
515516
proc = NULL; /* keep compiler quiet */
@@ -536,48 +537,48 @@ range_gist_consistent_leaf(FmgrInfo *flinfo, StrategyNumber strategy,
536537

537538
switch (strategy)
538539
{
539-
case RANGESTRAT_EQ:
540-
proc = range_eq;
541-
break;
542-
case RANGESTRAT_NE:
543-
proc = range_ne;
544-
break;
545-
case RANGESTRAT_OVERLAPS:
546-
proc = range_overlaps;
547-
break;
548-
case RANGESTRAT_CONTAINS_ELEM:
549-
case RANGESTRAT_CONTAINS:
550-
proc = range_contains;
551-
break;
552-
case RANGESTRAT_ELEM_CONTAINED_BY:
553-
case RANGESTRAT_CONTAINED_BY:
554-
proc = range_contained_by;
555-
break;
556540
case RANGESTRAT_BEFORE:
557541
if (RangeIsEmpty(key) || RangeIsEmpty(query))
558542
return false;
559543
proc = range_before;
560544
break;
561-
case RANGESTRAT_AFTER:
562-
if (RangeIsEmpty(key) || RangeIsEmpty(query))
563-
return false;
564-
proc = range_after;
565-
break;
566545
case RANGESTRAT_OVERLEFT:
567546
if (RangeIsEmpty(key) || RangeIsEmpty(query))
568547
return false;
569548
proc = range_overleft;
570549
break;
550+
case RANGESTRAT_OVERLAPS:
551+
proc = range_overlaps;
552+
break;
571553
case RANGESTRAT_OVERRIGHT:
572554
if (RangeIsEmpty(key) || RangeIsEmpty(query))
573555
return false;
574556
proc = range_overright;
575557
break;
558+
case RANGESTRAT_AFTER:
559+
if (RangeIsEmpty(key) || RangeIsEmpty(query))
560+
return false;
561+
proc = range_after;
562+
break;
576563
case RANGESTRAT_ADJACENT:
577564
if (RangeIsEmpty(key) || RangeIsEmpty(query))
578565
return false;
579566
proc = range_adjacent;
580567
break;
568+
case RANGESTRAT_CONTAINS:
569+
case RANGESTRAT_CONTAINS_ELEM:
570+
proc = range_contains;
571+
break;
572+
case RANGESTRAT_CONTAINED_BY:
573+
case RANGESTRAT_ELEM_CONTAINED_BY:
574+
proc = range_contained_by;
575+
break;
576+
case RANGESTRAT_EQ:
577+
proc = range_eq;
578+
break;
579+
case RANGESTRAT_NE:
580+
proc = range_ne;
581+
break;
581582
default:
582583
elog(ERROR, "unrecognized range strategy: %d", strategy);
583584
proc = NULL; /* keep compiler quiet */

‎src/include/catalog/catversion.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,6 @@
5353
*/
5454

5555
/* yyyymmddN */
56-
#define CATALOG_VERSION_NO 201111171
56+
#define CATALOG_VERSION_NO 201111211
5757

5858
#endif

‎src/include/catalog/pg_amop.h

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -726,17 +726,17 @@ DATA(insert ( 3903 3831 3831 1 s 3882 405 0 ));
726726
/*
727727
* GiST range_ops
728728
*/
729-
DATA(insert ( 3919 3831 3831 1 s 3882 783 0 ));
730-
DATA(insert ( 3919 3831 3831 2 s 3883 783 0 ));
729+
DATA(insert ( 3919 3831 3831 1 s 3893 783 0 ));
730+
DATA(insert ( 3919 3831 3831 2 s 3895 783 0 ));
731731
DATA(insert ( 3919 3831 3831 3 s 3888 783 0 ));
732-
DATA(insert ( 3919 3831 2283 4 s 3889 783 0 ));
733-
DATA(insert ( 3919 2283 3831 5 s 3891 783 0 ));
734-
DATA(insert ( 3919 3831 3831 6 s 3890 783 0 ));
735-
DATA(insert ( 3919 3831 3831 7 s 3892 783 0 ));
736-
DATA(insert ( 3919 3831 3831 8 s 3893 783 0 ));
737-
DATA(insert ( 3919 3831 3831 9 s 3894 783 0 ));
738-
DATA(insert ( 3919 3831 3831 10 s 3895 783 0 ));
739-
DATA(insert ( 3919 3831 3831 11 s 3896 783 0 ));
740-
DATA(insert ( 3919 3831 3831 12 s 3897 783 0 ));
732+
DATA(insert ( 3919 3831 3831 4 s 3896 783 0 ));
733+
DATA(insert ( 3919 3831 3831 5 s 3894 783 0 ));
734+
DATA(insert ( 3919 3831 3831 6 s 3897 783 0 ));
735+
DATA(insert ( 3919 3831 3831 7 s 3890 783 0 ));
736+
DATA(insert ( 3919 3831 3831 8 s 3892 783 0 ));
737+
DATA(insert ( 3919 3831 2283 16 s 3889 783 0 ));
738+
DATA(insert ( 3919 2283 3831 17 s 3891 783 0 ));
739+
DATA(insert ( 3919 3831 3831 18 s 3882 783 0 ));
740+
DATA(insert ( 3919 3831 3831 19 s 3883 783 0 ));
741741

742742
#endif /* PG_AMOP_H */

‎src/include/catalog/pg_operator.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1674,15 +1674,15 @@ DATA(insert OID = 3886 ( ">=" PGNSP PGUID b f f 3831 3831 16 3885 3884 range
16741674
DESCR("greater than or equal");
16751675
DATA(insert OID = 3887 ( ">" PGNSP PGUID b f f 3831 3831 16 3884 3885 range_gt scalargtsel scalargtjoinsel ));
16761676
DESCR("greater than");
1677-
DATA(insert OID = 3888 ( "&&" PGNSP PGUID b f f 3831 3831 16 3888 0 range_overlaps - - ));
1677+
DATA(insert OID = 3888 ( "&&" PGNSP PGUID b f f 3831 3831 16 3888 0 range_overlaps areasel areajoinsel ));
16781678
DESCR("overlaps");
1679-
DATA(insert OID = 3889 ( "@>" PGNSP PGUID b f f 3831 2283 16 3891 0 range_contains_elem - - ));
1679+
DATA(insert OID = 3889 ( "@>" PGNSP PGUID b f f 3831 2283 16 3891 0 range_contains_elem contsel contjoinsel ));
16801680
DESCR("contains");
1681-
DATA(insert OID = 3890 ( "@>" PGNSP PGUID b f f 3831 3831 16 3892 0 range_contains - - ));
1681+
DATA(insert OID = 3890 ( "@>" PGNSP PGUID b f f 3831 3831 16 3892 0 range_contains contsel contjoinsel ));
16821682
DESCR("contains");
1683-
DATA(insert OID = 3891 ( "<@" PGNSP PGUID b f f 2283 3831 16 3889 0 elem_contained_by_range - - ));
1683+
DATA(insert OID = 3891 ( "<@" PGNSP PGUID b f f 2283 3831 16 3889 0 elem_contained_by_range contsel contjoinsel ));
16841684
DESCR("is contained by");
1685-
DATA(insert OID = 3892 ( "<@" PGNSP PGUID b f f 3831 3831 16 3890 0 range_contained_by - - ));
1685+
DATA(insert OID = 3892 ( "<@" PGNSP PGUID b f f 3831 3831 16 3890 0 range_contained_by contsel contjoinsel ));
16861686
DESCR("is contained by");
16871687
DATA(insert OID = 3893 ( "<<" PGNSP PGUID b f f 3831 3831 16 3894 0 range_before scalarltsel scalarltjoinsel ));
16881688
DESCR("is left of");
@@ -1692,7 +1692,7 @@ DATA(insert OID = 3895 ( "&<" PGNSP PGUID b f f 3831 3831 16 0 0 range_overl
16921692
DESCR("overlaps or is left of");
16931693
DATA(insert OID = 3896 ( "&>" PGNSP PGUID b f f 3831 3831 16 0 0 range_overright scalargtsel scalargtjoinsel ));
16941694
DESCR("overlaps or is right of");
1695-
DATA(insert OID = 3897 ( "-|-" PGNSP PGUID b f f 3831 3831 16 3897 0 range_adjacent - - ));
1695+
DATA(insert OID = 3897 ( "-|-" PGNSP PGUID b f f 3831 3831 16 3897 0 range_adjacent contsel contjoinsel ));
16961696
DESCR("is adjacent to");
16971697
DATA(insert OID = 3898 ( "+" PGNSP PGUID b f f 3831 3831 3831 3898 0 range_union - - ));
16981698
DESCR("range union");

‎src/test/regress/expected/opr_sanity.out

Lines changed: 19 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -140,16 +140,16 @@ WHERE p1.oid < p2.oid AND
140140
-- need to be modified whenever new pairs of types are made binary-equivalent,
141141
-- or when new polymorphic built-in functions are added!
142142
-- Note: ignore aggregate functions here, since they all point to the same
143-
-- dummy built-in function.
143+
-- dummy built-in function. Likewise, ignore range constructor functions.
144144
SELECT DISTINCT p1.prorettype, p2.prorettype
145145
FROM pg_proc AS p1, pg_proc AS p2
146146
WHERE p1.oid != p2.oid AND
147147
p1.prosrc = p2.prosrc AND
148148
p1.prolang = 12 AND p2.prolang = 12 AND
149149
NOT p1.proisagg AND NOT p2.proisagg AND
150-
(p1.prorettype < p2.prorettype) AND
151-
-- range constructor functions are shared by all range types.
152-
NOT p1.prosrc LIKE 'range_constructor%'
150+
p1.prosrc NOT LIKE E'range\\_constructor_' AND
151+
p2.prosrc NOT LIKE E'range\\_constructor_' AND
152+
(p1.prorettype < p2.prorettype)
153153
ORDER BY 1, 2;
154154
prorettype | prorettype
155155
------------+------------
@@ -163,9 +163,9 @@ WHERE p1.oid != p2.oid AND
163163
p1.prosrc = p2.prosrc AND
164164
p1.prolang = 12 AND p2.prolang = 12 AND
165165
NOT p1.proisagg AND NOT p2.proisagg AND
166-
(p1.proargtypes[0] < p2.proargtypes[0]) AND
167-
-- range constructor functions are shared by all range types.
168-
NOT p1.prosrc LIKE 'range_constructor%'
166+
p1.prosrc NOT LIKE E'range\\_constructor_' AND
167+
p2.prosrc NOT LIKE E'range\\_constructor_' AND
168+
(p1.proargtypes[0] < p2.proargtypes[0])
169169
ORDER BY 1, 2;
170170
proargtypes | proargtypes
171171
-------------+-------------
@@ -182,9 +182,9 @@ WHERE p1.oid != p2.oid AND
182182
p1.prosrc = p2.prosrc AND
183183
p1.prolang = 12 AND p2.prolang = 12 AND
184184
NOT p1.proisagg AND NOT p2.proisagg AND
185-
(p1.proargtypes[1] < p2.proargtypes[1]) AND
186-
-- range constructor functions are shared by all range types.
187-
NOT p1.prosrc LIKE 'range_constructor%'
185+
p1.prosrc NOT LIKE E'range\\_constructor_' AND
186+
p2.prosrc NOT LIKE E'range\\_constructor_' AND
187+
(p1.proargtypes[1] < p2.proargtypes[1])
188188
ORDER BY 1, 2;
189189
proargtypes | proargtypes
190190
-------------+-------------
@@ -1021,34 +1021,28 @@ ORDER BY 1, 2, 3;
10211021
403 | 5 | ~>~
10221022
405 | 1 | =
10231023
783 | 1 | <<
1024-
783 | 1 | =
10251024
783 | 1 | @@
10261025
783 | 2 | &<
1027-
783 | 2 | <>
10281026
783 | 3 | &&
10291027
783 | 4 | &>
1030-
783 | 4 | @>
1031-
783 | 5 | <@
10321028
783 | 5 | >>
1033-
783 | 6 | @>
1029+
783 | 6 | -|-
10341030
783 | 6 | ~=
1035-
783 | 7 | <@
10361031
783 | 7 | @>
1037-
783 | 8 | <<
10381032
783 | 8 | <@
10391033
783 | 9 | &<|
1040-
783 | 9 | >>
1041-
783 | 10 | &<
10421034
783 | 10 | <<|
10431035
783 | 10 | <^
1044-
783 | 11 | &>
10451036
783 | 11 | >^
10461037
783 | 11 | |>>
1047-
783 | 12 | -|-
10481038
783 | 12 | |&>
10491039
783 | 13 | ~
10501040
783 | 14 | @
10511041
783 | 15 | <->
1042+
783 | 16 | @>
1043+
783 | 17 | <@
1044+
783 | 18 | =
1045+
783 | 19 | <>
10521046
783 | 27 | @>
10531047
783 | 28 | <@
10541048
783 | 47 | @>
@@ -1061,7 +1055,7 @@ ORDER BY 1, 2, 3;
10611055
2742 | 2 | @@@
10621056
2742 | 3 | <@
10631057
2742 | 4 | =
1064-
(51 rows)
1058+
(45 rows)
10651059

10661060
-- Check that all opclass search operators have selectivity estimators.
10671061
-- This is not absolutely required, but it seems a reasonable thing
@@ -1070,15 +1064,9 @@ SELECT p1.amopfamily, p1.amopopr, p2.oid, p2.oprname
10701064
FROM pg_amop AS p1, pg_operator AS p2
10711065
WHERE p1.amopopr = p2.oid AND p1.amoppurpose = 's' AND
10721066
(p2.oprrest = 0 OR p2.oprjoin = 0);
1073-
amopfamily | amopopr | oid | oprname
1074-
------------+---------+------+---------
1075-
3919 | 3888 | 3888 | &&
1076-
3919 | 3889 | 3889 | @>
1077-
3919 | 3891 | 3891 | <@
1078-
3919 | 3890 | 3890 | @>
1079-
3919 | 3892 | 3892 | <@
1080-
3919 | 3897 | 3897 | -|-
1081-
(6 rows)
1067+
amopfamily | amopopr | oid | oprname
1068+
------------+---------+-----+---------
1069+
(0 rows)
10821070

10831071
-- Check that each opclass in an opfamily has associated operators, that is
10841072
-- ones whose oprleft matches opcintype (possibly by coercion).

‎src/test/regress/expected/plpgsql.out

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4571,17 +4571,3 @@ ERROR: value for domain orderedarray violates check constraint "sorted"
45714571
CONTEXT: PL/pgSQL function "testoa" line 5 at assignment
45724572
drop function arrayassign1();
45734573
drop function testoa(x1 int, x2 int, x3 int);
4574-
-- Test resolve_polymorphic_argtypes() codepath. It is only taken when
4575-
-- a function is invoked from a different backend from where it's defined,
4576-
-- so we create the a function with polymorphic argument, reconnect, and
4577-
-- and then call it.
4578-
create function rangetypes_plpgsql(out a anyelement, b anyrange, c anyarray)
4579-
language plpgsql as
4580-
$$ begin a := upper(b) + c[1]; return; end; $$;
4581-
\c -
4582-
select rangetypes_plpgsql(int4range(1,10),ARRAY[2,20]);
4583-
rangetypes_plpgsql
4584-
--------------------
4585-
12
4586-
(1 row)
4587-

0 commit comments

Comments
 (0)