Re: allow frontend use of the backend's core hashing functions

Mark Dilger <mark.dilger@enterprisedb.com>

From: Mark Dilger <mark.dilger@enterprisedb.com>
To: Suraj Kharage <suraj.kharage@enterprisedb.com>
Cc: Robert Haas <robertmhaas@gmail.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2020-02-13T16:26:49Z
Lists: pgsql-hackers

Attachments


> On Feb 13, 2020, at 3:44 AM, Suraj Kharage <suraj.kharage@enterprisedb.com> wrote:
> 
> Hi,
> 
> I have spent some time reviewing the patches and overall it looks good to me.
> 
> However, I have few cosmetic review comments for 0003 patch as below;
> 
> 1: 
> +++ b/src/backend/utils/hash/hashfn.c
> @@ -16,15 +16,14 @@
>   *	 It is expected that every bit of a hash function's 32-bit result is
>   *	 as random as every other; failure to ensure this is likely to lead
>   *	 to poor performance of hash tables.  In most cases a hash
> - *	 function should use hash_any() or its variant hash_uint32().
> + *	 function should use hash_bytes() or its variant hash_bytes_uint32(),
> + *	 or the wrappers hash_any() and hash_any_uint32 defined in hashfn.h.
> 
> Here, indicated function name should be hash_uint32.

+1

> 2: I can see renamed functions are declared twice in hashutils.c. I think duplicate declarations after #endif can be removed,
> 
> +extern uint32 hash_bytes(const unsigned char *k, int keylen);
> +extern uint64 hash_bytes_extended(const unsigned char *k,
> +	 int keylen, uint64 seed);
> +extern uint32 hash_bytes_uint32(uint32 k);
> +extern uint64 hash_bytes_uint32_extended(uint32 k, uint64 seed);
> +
> +#ifndef FRONTEND
> ..
> Wrapper functions
> ..
> +#endif
> +
> +extern uint32 hash_bytes(const unsigned char *k, int keylen);
> +extern uint64 hash_bytes_extended(const unsigned char *k,
> +	 int keylen, uint64 seed);
> +extern uint32 hash_bytes_uint32(uint32 k);
> +extern uint64 hash_bytes_uint32_extended(uint32 k, uint64 seed);

+1

> 3: The first line of the commit message has one typo.
> defiend => defined.

+1

I have made these changes and rebased Robert’s patches but otherwise changed nothing.  Here they are:

Commits

  1. Move src/backend/utils/hash/hashfn.c to src/common

  2. Adapt hashfn.c and hashutils.h for frontend use.

  3. Put all the prototypes for hashfn.c into the same header file.

  4. Move bitmap_hash and bitmap_match to bitmapset.c.