]> granicus.if.org Git - postgresql/commitdiff
Fix typcache's failure to treat ranges as container types.
authorTom Lane <tgl@sss.pgh.pa.us>
Fri, 20 Oct 2017 21:12:27 +0000 (17:12 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Fri, 20 Oct 2017 21:12:27 +0000 (17:12 -0400)
Like the similar logic for arrays and records, it's necessary to examine
the range's subtype to decide whether the range type can support hashing.
We can omit checking the subtype for btree-defined operations, though,
since range subtypes are required to have those operations.  (Possibly
that simplification for btree cases led us to overlook that it does
not apply for hash cases.)

This is only an issue if the subtype lacks hash support, which is not
true of any built-in range type, but it's easy to demonstrate a problem
with a range type over, eg, money: you can get a "could not identify
a hash function" failure when the planner is misled into thinking that
hash join or aggregation would work.

This was born broken, so back-patch to all supported branches.

src/backend/utils/cache/typcache.c
src/test/regress/expected/rangetypes.out
src/test/regress/sql/rangetypes.sql

index a0a71dd86a89370b646f88accba0a442c02653f2..61ce7dc2a7b184fcddc63e20beac2f97d837ac82 100644 (file)
@@ -279,6 +279,9 @@ static void cache_array_element_properties(TypeCacheEntry *typentry);
 static bool record_fields_have_equality(TypeCacheEntry *typentry);
 static bool record_fields_have_compare(TypeCacheEntry *typentry);
 static void cache_record_field_properties(TypeCacheEntry *typentry);
+static bool range_element_has_hashing(TypeCacheEntry *typentry);
+static bool range_element_has_extended_hashing(TypeCacheEntry *typentry);
+static void cache_range_element_properties(TypeCacheEntry *typentry);
 static void TypeCacheRelCallback(Datum arg, Oid relid);
 static void TypeCacheOpcCallback(Datum arg, int cacheid, uint32 hashvalue);
 static void TypeCacheConstrCallback(Datum arg, int cacheid, uint32 hashvalue);
@@ -480,9 +483,11 @@ lookup_type_cache(Oid type_id, int flags)
 
                /*
                 * If the proposed equality operator is array_eq or record_eq, check
-                * to see if the element type or column types support equality. If
+                * to see if the element type or column types support equality.  If
                 * not, array_eq or record_eq would fail at runtime, so we don't want
-                * to report that the type has equality.
+                * to report that the type has equality.  (We can omit similar
+                * checking for ranges because ranges can't be created in the first
+                * place unless their subtypes support equality.)
                 */
                if (eq_opr == ARRAY_EQ_OP &&
                        !array_element_has_equality(typentry))
@@ -517,7 +522,10 @@ lookup_type_cache(Oid type_id, int flags)
                                                                                 typentry->btree_opintype,
                                                                                 BTLessStrategyNumber);
 
-               /* As above, make sure array_cmp or record_cmp will succeed */
+               /*
+                * As above, make sure array_cmp or record_cmp will succeed; but again
+                * we need no special check for ranges.
+                */
                if (lt_opr == ARRAY_LT_OP &&
                        !array_element_has_compare(typentry))
                        lt_opr = InvalidOid;
@@ -539,7 +547,10 @@ lookup_type_cache(Oid type_id, int flags)
                                                                                 typentry->btree_opintype,
                                                                                 BTGreaterStrategyNumber);
 
-               /* As above, make sure array_cmp or record_cmp will succeed */
+               /*
+                * As above, make sure array_cmp or record_cmp will succeed; but again
+                * we need no special check for ranges.
+                */
                if (gt_opr == ARRAY_GT_OP &&
                        !array_element_has_compare(typentry))
                        gt_opr = InvalidOid;
@@ -561,7 +572,10 @@ lookup_type_cache(Oid type_id, int flags)
                                                                                 typentry->btree_opintype,
                                                                                 BTORDER_PROC);
 
-               /* As above, make sure array_cmp or record_cmp will succeed */
+               /*
+                * As above, make sure array_cmp or record_cmp will succeed; but again
+                * we need no special check for ranges.
+                */
                if (cmp_proc == F_BTARRAYCMP &&
                        !array_element_has_compare(typentry))
                        cmp_proc = InvalidOid;
@@ -605,6 +619,13 @@ lookup_type_cache(Oid type_id, int flags)
                        !array_element_has_hashing(typentry))
                        hash_proc = InvalidOid;
 
+               /*
+                * Likewise for hash_range.
+                */
+               if (hash_proc == F_HASH_RANGE &&
+                       !range_element_has_hashing(typentry))
+                       hash_proc = InvalidOid;
+
                /* Force update of hash_proc_finfo only if we're changing state */
                if (typentry->hash_proc != hash_proc)
                        typentry->hash_proc_finfo.fn_oid = InvalidOid;
@@ -642,6 +663,13 @@ lookup_type_cache(Oid type_id, int flags)
                        !array_element_has_extended_hashing(typentry))
                        hash_extended_proc = InvalidOid;
 
+               /*
+                * Likewise for hash_range_extended.
+                */
+               if (hash_extended_proc == F_HASH_RANGE_EXTENDED &&
+                       !range_element_has_extended_hashing(typentry))
+                       hash_extended_proc = InvalidOid;
+
                /* Force update of proc finfo only if we're changing state */
                if (typentry->hash_extended_proc != hash_extended_proc)
                        typentry->hash_extended_proc_finfo.fn_oid = InvalidOid;
@@ -1305,6 +1333,10 @@ cache_array_element_properties(TypeCacheEntry *typentry)
        typentry->flags |= TCFLAGS_CHECKED_ELEM_PROPERTIES;
 }
 
+/*
+ * Likewise, some helper functions for composite types.
+ */
+
 static bool
 record_fields_have_equality(TypeCacheEntry *typentry)
 {
@@ -1376,6 +1408,54 @@ cache_record_field_properties(TypeCacheEntry *typentry)
        typentry->flags |= TCFLAGS_CHECKED_FIELD_PROPERTIES;
 }
 
+/*
+ * Likewise, some helper functions for range types.
+ *
+ * We can borrow the flag bits for array element properties to use for range
+ * element properties, since those flag bits otherwise have no use in a
+ * range type's typcache entry.
+ */
+
+static bool
+range_element_has_hashing(TypeCacheEntry *typentry)
+{
+       if (!(typentry->flags & TCFLAGS_CHECKED_ELEM_PROPERTIES))
+               cache_range_element_properties(typentry);
+       return (typentry->flags & TCFLAGS_HAVE_ELEM_HASHING) != 0;
+}
+
+static bool
+range_element_has_extended_hashing(TypeCacheEntry *typentry)
+{
+       if (!(typentry->flags & TCFLAGS_CHECKED_ELEM_PROPERTIES))
+               cache_range_element_properties(typentry);
+       return (typentry->flags & TCFLAGS_HAVE_ELEM_EXTENDED_HASHING) != 0;
+}
+
+static void
+cache_range_element_properties(TypeCacheEntry *typentry)
+{
+       /* load up subtype link if we didn't already */
+       if (typentry->rngelemtype == NULL &&
+               typentry->typtype == TYPTYPE_RANGE)
+               load_rangetype_info(typentry);
+
+       if (typentry->rngelemtype != NULL)
+       {
+               TypeCacheEntry *elementry;
+
+               /* might need to calculate subtype's hash function properties */
+               elementry = lookup_type_cache(typentry->rngelemtype->type_id,
+                                                                         TYPECACHE_HASH_PROC |
+                                                                         TYPECACHE_HASH_EXTENDED_PROC);
+               if (OidIsValid(elementry->hash_proc))
+                       typentry->flags |= TCFLAGS_HAVE_ELEM_HASHING;
+               if (OidIsValid(elementry->hash_extended_proc))
+                       typentry->flags |= TCFLAGS_HAVE_ELEM_EXTENDED_HASHING;
+       }
+       typentry->flags |= TCFLAGS_CHECKED_ELEM_PROPERTIES;
+}
+
 /*
  * Make sure that RecordCacheArray is large enough to store 'typmod'.
  */
index 4a2336cd8d3a7dbc718c9e79525c4f3c692e994e..accf1e0d9e25f1cfd84e0b89d1c211c0db1cb0f6 100644 (file)
@@ -1354,6 +1354,18 @@ select *, row_to_json(upper(t)) as u from
 drop type two_ints cascade;
 NOTICE:  drop cascades to type two_ints_range
 --
+-- Check behavior when subtype lacks a hash function
+--
+create type cashrange as range (subtype = money);
+set enable_sort = off;  -- try to make it pick a hash setop implementation
+select '(2,5)'::cashrange except select '(5,6)'::cashrange;
+   cashrange   
+---------------
+ ($2.00,$5.00)
+(1 row)
+
+reset enable_sort;
+--
 -- OUT/INOUT/TABLE functions
 --
 create function outparam_succeed(i anyrange, out r anyrange, out t text)
index a60df9095e99435ee3e8c1a1328f3563f8511569..55638a85ee1308a6b48d8bcba9881cb2100eab4a 100644 (file)
@@ -461,6 +461,18 @@ select *, row_to_json(upper(t)) as u from
 
 drop type two_ints cascade;
 
+--
+-- Check behavior when subtype lacks a hash function
+--
+
+create type cashrange as range (subtype = money);
+
+set enable_sort = off;  -- try to make it pick a hash setop implementation
+
+select '(2,5)'::cashrange except select '(5,6)'::cashrange;
+
+reset enable_sort;
+
 --
 -- OUT/INOUT/TABLE functions
 --