Fix float4/float8 hash functions to produce uniform results for NaNs.

The IEEE 754 standard allows a wide variety of bit patterns for NaNs,
of which at least two ("NaN" and "-NaN") are pretty easy to produce
from SQL on most machines.  This is problematic because our btree
comparison functions deem all NaNs to be equal, but our float hash
functions know nothing about NaNs and will happily produce varying
hash codes for them.  That causes unexpected results from queries
that hash a column containing different NaN values.  It could also
produce unexpected lookup failures when using a hash index on a
float column, i.e. "WHERE x = 'NaN'" will not find all the rows
it should.

To fix, special-case NaN in the float hash functions, not too much
unlike the existing special case that forces zero and minus zero
to hash the same.  I arranged for the most vanilla sort of NaN
(that coming from the C99 NAN constant) to still have the same
hash code as before, to reduce the risk to existing hash indexes.

I dithered about whether to back-patch this into stable branches,
but ultimately decided to do so.  It's a clear improvement for
queries that hash internally.  If there is anybody who has -NaN
in a hash index, they'd be well advised to re-index after applying
this patch ... but the misbehavior if they don't will not be much
worse than the misbehavior they had before.

Per bug #17172 from Ma Liangzhu.

Discussion: https://postgr.es/m/17172-7505bea9e04e230f@postgresql.org

diff --git a/src/backend/access/hash/hashfunc.c b/src/backend/access/hash/hashfunc.c
index db20d9d1c1456f6d7ef24ee2f4d8f7e7b50671cd..68c3ebb9bba3feaa208bccd29160a1cb7fa9ca01 100644 (file)
--- a/src/backend/access/hash/hashfunc.c
+++ b/src/backend/access/hash/hashfunc.c
@@ -30,6 +30,7 @@
 #include "catalog/pg_collation.h"
 #include "common/hashfn.h"
 #include "utils/builtins.h"
+#include "utils/float.h"
 #include "utils/pg_locale.h"
 
 /*
@@ -150,6 +151,14 @@ hashfloat4(PG_FUNCTION_ARGS)
    if (key == (float4) 0)
        PG_RETURN_UINT32(0);
 
+   /*
+    * Similarly, NaNs can have different bit patterns but they should all
+    * compare as equal.  For backwards-compatibility reasons we force them to
+    * have the hash value of a standard NaN.
+    */
+   if (isnan(key))
+       key = get_float4_nan();
+
    /*
     * To support cross-type hashing of float8 and float4, we want to return
     * the same hash value hashfloat8 would produce for an equal float8 value.
@@ -172,6 +181,8 @@ hashfloat4extended(PG_FUNCTION_ARGS)
    /* Same approach as hashfloat4 */
    if (key == (float4) 0)
        PG_RETURN_UINT64(seed);
+   if (isnan(key))
+       key = get_float4_nan();
    key8 = key;
 
    return hash_any_extended((unsigned char *) &key8, sizeof(key8), seed);
@@ -190,6 +201,14 @@ hashfloat8(PG_FUNCTION_ARGS)
    if (key == (float8) 0)
        PG_RETURN_UINT32(0);
 
+   /*
+    * Similarly, NaNs can have different bit patterns but they should all
+    * compare as equal.  For backwards-compatibility reasons we force them to
+    * have the hash value of a standard NaN.
+    */
+   if (isnan(key))
+       key = get_float8_nan();
+
    return hash_any((unsigned char *) &key, sizeof(key));
 }
 
@@ -202,6 +221,8 @@ hashfloat8extended(PG_FUNCTION_ARGS)
    /* Same approach as hashfloat8 */
    if (key == (float8) 0)
        PG_RETURN_UINT64(seed);
+   if (isnan(key))
+       key = get_float8_nan();
 
    return hash_any_extended((unsigned char *) &key, sizeof(key), seed);
 }

diff --git a/src/test/regress/expected/hash_func.out b/src/test/regress/expected/hash_func.out
index dca31365bcfce4390b65bf85e5c81b158cd0ee6a..61c6cbbf109a8bd3f18c019309d8b4e1d4bed175 100644 (file)
--- a/src/test/regress/expected/hash_func.out
+++ b/src/test/regress/expected/hash_func.out
@@ -339,3 +339,36 @@ SELECT v as value, hash_record_extended(v, 0)::bit(32) as extended0
 FROM   (VALUES (row(1, 'aaa')::hash_test_t2)) x(v);
 ERROR:  could not identify an extended hash function for type money
 DROP TYPE hash_test_t2;
+--
+-- Check special cases for specific data types
+--
+SELECT hashfloat4('0'::float4) = hashfloat4('-0'::float4) AS t;
+ t 
+---
+ t
+(1 row)
+
+SELECT hashfloat4('NaN'::float4) = hashfloat4('-NaN'::float4) AS t;
+ t 
+---
+ t
+(1 row)
+
+SELECT hashfloat8('0'::float8) = hashfloat8('-0'::float8) AS t;
+ t 
+---
+ t
+(1 row)
+
+SELECT hashfloat8('NaN'::float8) = hashfloat8('-NaN'::float8) AS t;
+ t 
+---
+ t
+(1 row)
+
+SELECT hashfloat4('NaN'::float4) = hashfloat8('NaN'::float8) AS t;
+ t 
+---
+ t
+(1 row)
+

diff --git a/src/test/regress/sql/hash_func.sql b/src/test/regress/sql/hash_func.sql
index 0292dedd15ae5076d170ff0e9ba8aa212954e172..5c994f5de5e40f68491eeb678a302c89f806c3d8 100644 (file)
--- a/src/test/regress/sql/hash_func.sql
+++ b/src/test/regress/sql/hash_func.sql
@@ -253,3 +253,12 @@ FROM   (VALUES (row(1, 'aaa')::hash_test_t2)) x(v);
 SELECT v as value, hash_record_extended(v, 0)::bit(32) as extended0
 FROM   (VALUES (row(1, 'aaa')::hash_test_t2)) x(v);
 DROP TYPE hash_test_t2;
+
+--
+-- Check special cases for specific data types
+--
+SELECT hashfloat4('0'::float4) = hashfloat4('-0'::float4) AS t;
+SELECT hashfloat4('NaN'::float4) = hashfloat4('-NaN'::float4) AS t;
+SELECT hashfloat8('0'::float8) = hashfloat8('-0'::float8) AS t;
+SELECT hashfloat8('NaN'::float8) = hashfloat8('-NaN'::float8) AS t;
+SELECT hashfloat4('NaN'::float4) = hashfloat8('NaN'::float8) AS t;