Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Fix CheckAttributeType's handling of collations for ranges.
authorTom Lane <tgl@sss.pgh.pa.us>
Fri, 31 Jan 2020 22:03:55 +0000 (17:03 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Fri, 31 Jan 2020 22:03:55 +0000 (17:03 -0500)
Commit fc7695891 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

src/backend/catalog/heap.c
src/backend/utils/cache/lsyscache.c
src/include/utils/lsyscache.h
src/test/regress/expected/rangetypes.out
src/test/regress/expected/sanity_check.out
src/test/regress/sql/rangetypes.sql

index 910e73f7eab7a5db74c504e2fae49fb01f9baf78..7d0e54a3b0a719c8ea4be5e23a2db3dfe55a06ff 100644 (file)
@@ -568,7 +568,8 @@ CheckAttributeType(const char *attname,
        /*
         * If it's a range, recurse to check its subtype.
         */
-       CheckAttributeType(attname, get_range_subtype(atttypid), attcollation,
+       CheckAttributeType(attname, get_range_subtype(atttypid),
+                          get_range_collation(atttypid),
                           containing_rowtypes,
                           allow_system_table_mods);
    }
index 37886945ff0712012cdaae261f3837f8f8d76cc4..b0b024dc3ed9674c5c1305e6b34d21c0275134ce 100644 (file)
@@ -3019,3 +3019,29 @@ get_range_subtype(Oid rangeOid)
    else
        return InvalidOid;
 }
+
+/*
+ * get_range_collation
+ *     Returns the collation of a given range type
+ *
+ * Returns InvalidOid if the type is not a range type,
+ * or if its subtype is not collatable.
+ */
+Oid
+get_range_collation(Oid rangeOid)
+{
+   HeapTuple   tp;
+
+   tp = SearchSysCache1(RANGETYPE, ObjectIdGetDatum(rangeOid));
+   if (HeapTupleIsValid(tp))
+   {
+       Form_pg_range rngtup = (Form_pg_range) GETSTRUCT(tp);
+       Oid         result;
+
+       result = rngtup->rngcollation;
+       ReleaseSysCache(tp);
+       return result;
+   }
+   else
+       return InvalidOid;
+}
index 9e7c03388b62b34f6e6dea05b9e37d418a2e1e33..489b196ecf4f505c5dc8738186c52a29d6886b8f 100644 (file)
@@ -157,6 +157,7 @@ extern void free_attstatsslot(Oid atttype,
 extern char *get_namespace_name(Oid nspid);
 extern char *get_namespace_name_or_temp(Oid nspid);
 extern Oid get_range_subtype(Oid rangeOid);
+extern Oid get_range_collation(Oid rangeOid);
 
 #define type_is_array(typid)  (get_element_type(typid) != InvalidOid)
 /* type_is_array_domain accepts both plain arrays and domains over arrays */
index 1e3e8b038a5fe41f6737b38fd3962ada1e1ec313..4a4c165968c99963fb25367192e74a9518f17077 100644 (file)
@@ -583,8 +583,95 @@ select * from numrange_test natural join numrange_test2 order by nr;
 set enable_nestloop to default;
 set enable_hashjoin to default;
 set enable_mergejoin to default;
-DROP TABLE numrange_test;
+-- keep numrange_test around to help exercise dump/reload
 DROP TABLE numrange_test2;
+--
+-- Apply a subset of the above tests on a collatable type, too
+--
+CREATE TABLE textrange_test (tr textrange);
+create index textrange_test_btree on textrange_test(tr);
+INSERT INTO textrange_test VALUES('[,)');
+INSERT INTO textrange_test VALUES('["a",]');
+INSERT INTO textrange_test VALUES('[,"q")');
+INSERT INTO textrange_test VALUES(textrange('b', 'g'));
+INSERT INTO textrange_test VALUES('empty');
+INSERT INTO textrange_test VALUES(textrange('d', 'd', '[]'));
+SELECT tr, isempty(tr), lower(tr), upper(tr) FROM textrange_test;
+  tr   | isempty | lower | upper 
+-------+---------+-------+-------
+ (,)   | f       |       | 
+ [a,)  | f       | a     | 
+ (,q)  | f       |       | q
+ [b,g) | f       | b     | g
+ empty | t       |       | 
+ [d,d] | f       | d     | d
+(6 rows)
+
+SELECT tr, lower_inc(tr), lower_inf(tr), upper_inc(tr), upper_inf(tr) FROM textrange_test;
+  tr   | lower_inc | lower_inf | upper_inc | upper_inf 
+-------+-----------+-----------+-----------+-----------
+ (,)   | f         | t         | f         | t
+ [a,)  | t         | f         | f         | t
+ (,q)  | f         | t         | f         | f
+ [b,g) | t         | f         | f         | f
+ empty | f         | f         | f         | f
+ [d,d] | t         | f         | t         | f
+(6 rows)
+
+SELECT * FROM textrange_test WHERE range_contains(tr, textrange('f', 'fx'));
+  tr   
+-------
+ (,)
+ [a,)
+ (,q)
+ [b,g)
+(4 rows)
+
+SELECT * FROM textrange_test WHERE tr @> textrange('a', 'z');
+  tr  
+------
+ (,)
+ [a,)
+(2 rows)
+
+SELECT * FROM textrange_test WHERE range_contained_by(textrange('0','9'), tr);
+  tr  
+------
+ (,)
+ (,q)
+(2 rows)
+
+SELECT * FROM textrange_test WHERE 'e'::text <@ tr;
+  tr   
+-------
+ (,)
+ [a,)
+ (,q)
+ [b,g)
+(4 rows)
+
+select * from textrange_test where tr = 'empty';
+  tr   
+-------
+ empty
+(1 row)
+
+select * from textrange_test where tr = '("b","g")';
+ tr 
+----
+(0 rows)
+
+select * from textrange_test where tr = '["b","g")';
+  tr   
+-------
+ [b,g)
+(1 row)
+
+select * from textrange_test where tr < 'empty';
+ tr 
+----
+(0 rows)
+
 -- test canonical form for int4range
 select int4range(1, 10, '[]');
  int4range 
index 36c15598f069700bcd0ccd23f68ac820cb088d7f..881c8c49152c913afeb198d6f2c2872b6610bf0f 100644 (file)
@@ -81,6 +81,7 @@ num_exp_sqrt|t
 num_exp_sub|t
 num_input_test|f
 num_result|f
+numrange_test|t
 onek|t
 onek2|t
 path_tbl|f
@@ -165,6 +166,7 @@ test_range_spgist|t
 test_tsvector|f
 testjsonb|f
 text_tbl|f
+textrange_test|t
 time_tbl|f
 timestamp_tbl|f
 timestamptz_tbl|f
index b8030e5aca79a5157cd969a29f419cfc12e6a262..45055415eb4317bb23ee8e39dd7253b9384fab9d 100644 (file)
@@ -148,9 +148,37 @@ set enable_nestloop to default;
 set enable_hashjoin to default;
 set enable_mergejoin to default;
 
-DROP TABLE numrange_test;
+-- keep numrange_test around to help exercise dump/reload
 DROP TABLE numrange_test2;
 
+--
+-- Apply a subset of the above tests on a collatable type, too
+--
+
+CREATE TABLE textrange_test (tr textrange);
+create index textrange_test_btree on textrange_test(tr);
+
+INSERT INTO textrange_test VALUES('[,)');
+INSERT INTO textrange_test VALUES('["a",]');
+INSERT INTO textrange_test VALUES('[,"q")');
+INSERT INTO textrange_test VALUES(textrange('b', 'g'));
+INSERT INTO textrange_test VALUES('empty');
+INSERT INTO textrange_test VALUES(textrange('d', 'd', '[]'));
+
+SELECT tr, isempty(tr), lower(tr), upper(tr) FROM textrange_test;
+SELECT tr, lower_inc(tr), lower_inf(tr), upper_inc(tr), upper_inf(tr) FROM textrange_test;
+
+SELECT * FROM textrange_test WHERE range_contains(tr, textrange('f', 'fx'));
+SELECT * FROM textrange_test WHERE tr @> textrange('a', 'z');
+SELECT * FROM textrange_test WHERE range_contained_by(textrange('0','9'), tr);
+SELECT * FROM textrange_test WHERE 'e'::text <@ tr;
+
+select * from textrange_test where tr = 'empty';
+select * from textrange_test where tr = '("b","g")';
+select * from textrange_test where tr = '["b","g")';
+select * from textrange_test where tr < 'empty';
+
+
 -- test canonical form for int4range
 select int4range(1, 10, '[]');
 select int4range(1, 10, '[)');