Skip to content

Commit f521ef0

Browse files
tglsfdctglsfdc
committed
Fix CheckAttributeType's handling of collations for ranges.
Commit fc76958 changed CheckAttributeType to recurse into ranges, but made it pass down the wrong collation (always InvalidOid, since ranges as such have no collation). This would result in guaranteed failure when considering a range type whose subtype is collatable. Embarrassingly, we lack any regression tests that would expose such a problem (but fortunately, somebody noticed before we shipped this bug in any release). Fix it to pass down the range's subtype collation property instead, and add some regression test cases to exercise collatable-subtype ranges a bit more. Back-patch to all supported branches, as the previous patch was. Report and patch by Julien Rouhaud, test cases tweaked by me Discussion: https://postgr.es/m/CAOBaU_aBWqNweiGUFX0guzBKkcfJ8mnnyyGC_KBQmO12Mj5f_A@mail.gmail.com
1 parent 5d60df8 commit f521ef0

File tree

6 files changed

+148
-3
lines changed

6 files changed

+148
-3
lines changed

‎src/backend/catalog/heap.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -566,7 +566,8 @@ CheckAttributeType(const char *attname,
566566
/*
567567
* If it's a range, recurse to check its subtype.
568568
*/
569-
CheckAttributeType(attname, get_range_subtype(atttypid), attcollation,
569+
CheckAttributeType(attname, get_range_subtype(atttypid),
570+
get_range_collation(atttypid),
570571
containing_rowtypes,
571572
allow_system_table_mods);
572573
}

‎src/backend/utils/cache/lsyscache.c

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2968,3 +2968,29 @@ get_range_subtype(Oid rangeOid)
29682968
else
29692969
return InvalidOid;
29702970
}
2971+
2972+
/*
2973+
* get_range_collation
2974+
* Returns the collation of a given range type
2975+
*
2976+
* Returns InvalidOid if the type is not a range type,
2977+
* or if its subtype is not collatable.
2978+
*/
2979+
Oid
2980+
get_range_collation(Oid rangeOid)
2981+
{
2982+
HeapTuple tp;
2983+
2984+
tp = SearchSysCache1(RANGETYPE, ObjectIdGetDatum(rangeOid));
2985+
if (HeapTupleIsValid(tp))
2986+
{
2987+
Form_pg_range rngtup = (Form_pg_range) GETSTRUCT(tp);
2988+
Oid result;
2989+
2990+
result = rngtup->rngcollation;
2991+
ReleaseSysCache(tp);
2992+
return result;
2993+
}
2994+
else
2995+
return InvalidOid;
2996+
}

‎src/include/utils/lsyscache.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@ extern void free_attstatsslot(Oid atttype,
154154
float4 *numbers, int nnumbers);
155155
extern char *get_namespace_name(Oid nspid);
156156
extern Oid get_range_subtype(Oid rangeOid);
157+
extern Oid get_range_collation(Oid rangeOid);
157158

158159
#define type_is_array(typid) (get_element_type(typid) != InvalidOid)
159160
/* type_is_array_domain accepts both plain arrays and domains over arrays */

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

Lines changed: 88 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -564,8 +564,95 @@ select * from numrange_test natural join numrange_test2 order by nr;
564564
set enable_nestloop to default;
565565
set enable_hashjoin to default;
566566
set enable_mergejoin to default;
567-
DROP TABLE numrange_test;
567+
-- keep numrange_test around to help exercise dump/reload
568568
DROP TABLE numrange_test2;
569+
--
570+
-- Apply a subset of the above tests on a collatable type, too
571+
--
572+
CREATE TABLE textrange_test (tr textrange);
573+
create index textrange_test_btree on textrange_test(tr);
574+
INSERT INTO textrange_test VALUES('[,)');
575+
INSERT INTO textrange_test VALUES('["a",]');
576+
INSERT INTO textrange_test VALUES('[,"q")');
577+
INSERT INTO textrange_test VALUES(textrange('b', 'g'));
578+
INSERT INTO textrange_test VALUES('empty');
579+
INSERT INTO textrange_test VALUES(textrange('d', 'd', '[]'));
580+
SELECT tr, isempty(tr), lower(tr), upper(tr) FROM textrange_test;
581+
tr | isempty | lower | upper
582+
-------+---------+-------+-------
583+
(,) | f | |
584+
[a,) | f | a |
585+
(,q) | f | | q
586+
[b,g) | f | b | g
587+
empty | t | |
588+
[d,d] | f | d | d
589+
(6 rows)
590+
591+
SELECT tr, lower_inc(tr), lower_inf(tr), upper_inc(tr), upper_inf(tr) FROM textrange_test;
592+
tr | lower_inc | lower_inf | upper_inc | upper_inf
593+
-------+-----------+-----------+-----------+-----------
594+
(,) | f | t | f | t
595+
[a,) | t | f | f | t
596+
(,q) | f | t | f | f
597+
[b,g) | t | f | f | f
598+
empty | f | f | f | f
599+
[d,d] | t | f | t | f
600+
(6 rows)
601+
602+
SELECT * FROM textrange_test WHERE range_contains(tr, textrange('f', 'fx'));
603+
tr
604+
-------
605+
(,)
606+
[a,)
607+
(,q)
608+
[b,g)
609+
(4 rows)
610+
611+
SELECT * FROM textrange_test WHERE tr @> textrange('a', 'z');
612+
tr
613+
------
614+
(,)
615+
[a,)
616+
(2 rows)
617+
618+
SELECT * FROM textrange_test WHERE range_contained_by(textrange('0','9'), tr);
619+
tr
620+
------
621+
(,)
622+
(,q)
623+
(2 rows)
624+
625+
SELECT * FROM textrange_test WHERE 'e'::text <@ tr;
626+
tr
627+
-------
628+
(,)
629+
[a,)
630+
(,q)
631+
[b,g)
632+
(4 rows)
633+
634+
select * from textrange_test where tr = 'empty';
635+
tr
636+
-------
637+
empty
638+
(1 row)
639+
640+
select * from textrange_test where tr = '("b","g")';
641+
tr
642+
----
643+
(0 rows)
644+
645+
select * from textrange_test where tr = '["b","g")';
646+
tr
647+
-------
648+
[b,g)
649+
(1 row)
650+
651+
select * from textrange_test where tr < 'empty';
652+
tr
653+
----
654+
(0 rows)
655+
569656
-- test canonical form for int4range
570657
select int4range(1, 10, '[]');
571658
int4range

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ num_exp_sqrt|t
8181
num_exp_sub|t
8282
num_input_test|f
8383
num_result|f
84+
numrange_test|t
8485
onek|t
8586
onek2|t
8687
path_tbl|f
@@ -162,6 +163,7 @@ test_range_spgist|t
162163
test_tsvector|f
163164
testjsonb|f
164165
text_tbl|f
166+
textrange_test|t
165167
time_tbl|f
166168
timestamp_tbl|f
167169
timestamptz_tbl|f

‎src/test/regress/sql/rangetypes.sql

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,9 +144,37 @@ set enable_nestloop to default;
144144
set enable_hashjoin to default;
145145
set enable_mergejoin to default;
146146

147-
DROP TABLE numrange_test;
147+
-- keep numrange_test around to help exercise dump/reload
148148
DROP TABLE numrange_test2;
149149

150+
--
151+
-- Apply a subset of the above tests on a collatable type, too
152+
--
153+
154+
CREATE TABLE textrange_test (tr textrange);
155+
create index textrange_test_btree on textrange_test(tr);
156+
157+
INSERT INTO textrange_test VALUES('[,)');
158+
INSERT INTO textrange_test VALUES('["a",]');
159+
INSERT INTO textrange_test VALUES('[,"q")');
160+
INSERT INTO textrange_test VALUES(textrange('b', 'g'));
161+
INSERT INTO textrange_test VALUES('empty');
162+
INSERT INTO textrange_test VALUES(textrange('d', 'd', '[]'));
163+
164+
SELECT tr, isempty(tr), lower(tr), upper(tr) FROM textrange_test;
165+
SELECT tr, lower_inc(tr), lower_inf(tr), upper_inc(tr), upper_inf(tr) FROM textrange_test;
166+
167+
SELECT * FROM textrange_test WHERE range_contains(tr, textrange('f', 'fx'));
168+
SELECT * FROM textrange_test WHERE tr @> textrange('a', 'z');
169+
SELECT * FROM textrange_test WHERE range_contained_by(textrange('0','9'), tr);
170+
SELECT * FROM textrange_test WHERE 'e'::text <@ tr;
171+
172+
select * from textrange_test where tr = 'empty';
173+
select * from textrange_test where tr = '("b","g")';
174+
select * from textrange_test where tr = '["b","g")';
175+
select * from textrange_test where tr < 'empty';
176+
177+
150178
-- test canonical form for int4range
151179
select int4range(1, 10, '[]');
152180
select int4range(1, 10, '[)');

0 commit comments

Comments
 (0)