Re: 64-bit queryId?
Julien Rouhaud <rjuju123@gmail.com>
From: Julien Rouhaud <rjuju123@gmail.com>
To: Michael Paquier <michael.paquier@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>,
Alexander Korotkov <a.korotkov@postgrespro.ru>, Tom Lane <tgl@sss.pgh.pa.us>, Peter Geoghegan <pg@bowt.ie>,
"Joshua D. Drake" <jd@commandprompt.com>, Greg Stark <stark@mit.edu>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2017-10-18T13:20:06Z
Lists: pgsql-hackers
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.
Commits
-
pg_stat_statements: Add a comment about the dangers of padding bytes.
- 2959213bf33c 11.0 landed
-
pg_stat_statements: Widen query IDs from 32 bits to 64 bits.
- cff440d36869 11.0 landed