Re: Hash Functions

On Wed, Aug 30, 2017 at 10:43 AM, amul sul <sulamul(at)gmail(dot)com> wrote:
> Thanks for the suggestion, I have updated 0002-patch accordingly.
> Using this I found some strange behaviours as follow:
>
> 1) standard and extended0 output for the jsonb_hash case is not same.
> 2) standard and extended0 output for the hash_range case is not same when
> input
> is int4range(550274, 1550274) other case in the patch are fine. This can
> be
> reproducible with other values as well, for e.g. int8range(1570275,
> 208112489).
>
> Will look into this tomorrow.

Those sound like bugs in your patch. Specifically:

+ /* Same approach as hash_range */
+ result = hash_uint32_extended((uint32) flags, seed);
+ result ^= lower_hash;
+ result = (result << 1) | (result >> 63);
+ result ^= upper_hash;

That doesn't give compatible results. The easiest thing to do might
be to rotate the high 32 bits and the low 32 bits separately.
JsonbHashScalarValueExtended has the same problem. Maybe create a
helper function that does something like this (untested):

((x << 1) & UINT64COUNT(0xfffffffefffffffe)) | ((x >> 31) &
UINT64CONST(0x100000001))

> Another case which I want to discuss is, extended and standard version of
> hashfloat4, hashfloat8 & hash_numeric function will have the same output for
> zero
> value irrespective of seed value. Do you think we need a fix for this?

Yes, I think you should return the seed rather than 0 in the cases
where the current code hard-codes a 0 return. That will give the same
behavior in the seed == 0 case while cheaply getting us something a
bit different when there is a seed.

BTW, you should probably invent and use a PG_RETURN_UINT64 macro in this patch.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company