On Thu, Oct 12, 2017 at 2:46 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Thu, Oct 12, 2017 at 9:05 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>> On Wed, Oct 4, 2017 at 9:00 PM, Michael Paquier
>> <michael.paquier@gmail.com> wrote:
>>> v4 looks correct to me. Testing it through (pgbench and some custom
>>> queries) I have not spotted issues. If the final decision is to use
>>> 64-bit query IDs, then this patch could be pushed.
>>
>> Cool. Committed, thanks for the review.
>
> The final result looks fine for me. Thanks.
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.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers