Horton

Thread

Commits

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

  1. BUG #17172: NaN compare error in hash agg

    The Post Office <noreply@postgresql.org> — 2021-09-01T02:25:31Z

    The following bug has been logged on the website:

Bug reference:      17172
Logged by:          ma liangzhu
Email address:      ma100@hotmail.com
PostgreSQL version: 14beta1
Operating system:   centos7
Description:        

postgres=# select '-NaN'::float union select ('Infinity'::float +
'-Infinity') union select 'NaN';
 float8 
--------
    NaN
    NaN
(2 rows)

postgres=# set enable_hashagg =0;
SET
postgres=# select '-NaN'::float union select ('Infinity'::float +
'-Infinity') union select 'NaN';
 float8 
--------
    NaN
(1 row)

  2. Re: BUG #17172: NaN compare error in hash agg

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-09-01T15:40:36Z

    PG Bug reporting form <noreply@postgresql.org> writes:
> postgres=# select '-NaN'::float union select ('Infinity'::float +
> '-Infinity') union select 'NaN';
>  float8 
> --------
>     NaN
>     NaN
> (2 rows)

Hm.  The reason for that is that NaN and -NaN are different bitpatterns
(0x7ff8000000000000 vs 0xfff8000000000000) so they produce different
hash values.  (On my machine, Inf plus -Inf produces -NaN, which is
not what I'd have expected, but that's what I see.)

Since float8_eq considers all NaNs equal, this is clearly a bug in the
float hash functions.  It's simple enough to fix, along the same lines
that we special-case minus zero: check isnan() and return some fixed
value if so.  I'm inclined to do that like this:

Datum
hashfloat8(PG_FUNCTION_ARGS)
{
    float8        key = PG_GETARG_FLOAT8(0);

    /*
     * On IEEE-float machines, minus zero and zero have different bit patterns
     * but should compare as equal.  We must ensure that they have the same
     * hash value, which is most reliably done this way:
     */
    if (key == (float8) 0)
        PG_RETURN_UINT32(0);

+   /*
+    * Similarly, NaNs can have different bit patterns but should compare
+    * as equal.  For backwards-compatibility reasons we force them all to
+    * have the hash value of a standard NaN.
+    */
+    if (isnan(key))
+       key = get_float8_nan();
+
     return hash_any((unsigned char *) &key, sizeof(key));
}

This has been broken for a very long time.  Do we dare back-patch
the fix?  Given the lack of complaints, maybe fixing it in HEAD/v14
is enough.  OTOH, it's not likely that many people have hash indexes
containing minus NaNs, so maybe it's okay to back-patch.

			regards, tom lane

  3. Re: BUG #17172: NaN compare error in hash agg

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-09-02T21:28:04Z

    I wrote:
> This has been broken for a very long time.  Do we dare back-patch
> the fix?  Given the lack of complaints, maybe fixing it in HEAD/v14
> is enough.  OTOH, it's not likely that many people have hash indexes
> containing minus NaNs, so maybe it's okay to back-patch.

After further thought I concluded that there's little reason
not to back-patch.  If someone has -NaN in a hash index,
they'd need to re-index if they ever want to find that entry
again ... but it was already true that many queries would not
find that entry.  Hence, pushed to all branches.

			regards, tom lane