Re: 64-bit queryId?

On Wed, Oct 18, 2017 at 9:20 AM, Julien Rouhaud <rjuju123(at)gmail(dot)com> wrote:
> Sorry for replying so late, but I have a perhaps naive question about
> the hashtable handling with this new version.
>
> IIUC, the shared hash table is now created with HASH_BLOBS instead of
> HASH_FUNCTION, so since sizeof(pgssHashKey) != sizeof(uint32) the hash
> table will use tag_hash() to compute the hash key.
>
> tag_hash() uses all the bits present in the given struct, so this can
> be problematic if padding bits are not zeroed, which isn't garanted by
> C standard for local variable.
>
> WIth current pgssHashKey definition, there shouldn't be padding bits,
> so it should be safe. But I wonder if adding an explicit memset() of
> the key in pgss_store() could avoid extension authors to have
> duplicate entries if they rely on this code, or prevent future issue
> in the unlikely case of adding other fields to pgssHashKey.

I guess we should probably add additional comment to the definition of
pgssHashKey warning of the danger. I'm OK with adding a memset if
somebody can promise me it will get optimized away by all reasonably
commonly-used compilers, but I'm not that keen on adding more cycles
to protect against a hypothetical danger.

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