Horton

Re: Add comment explaining why queryid is int64 in pg_stat_statements

David Rowley <dgrowleyml@gmail.com>

View thread
From: David Rowley <dgrowleyml@gmail.com>
To: Sami Imseih <samimseih@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>, Lukas Fittl <lukas@fittl.com>, Peter Eisentraut <peter@eisentraut.org>, Shaik Mohammad Mujeeb <mujeeb.sk@zohocorp.com>, ilyaevdokimov <ilya.evdokimov@tantorlabs.com>, pgsql-hackers <pgsql-hackers@postgresql.org>, mujeebskdev <mujeeb.sk.dev@gmail.com>, pgsql-hackers <pgsql-hackers@lists.postgresql.org>
Date: 2025-05-30T11:08:58Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Change internal plan ID type from uint64 to int64

  2. Change internal queryid type from uint64 to int64

  3. pg_stat_statements: Widen query IDs from 32 bits to 64 bits.

Thanks for the review.

On Wed, 21 May 2025 at 03:35, Sami Imseih <samimseih@gmail.com> wrote:
> 2/ Should "DatumGetInt64(hash_any_extended(......" be turned into a
> macro since it's used in
> several places? Also, we can add a comment in the macro such as
> "
> Output the queryId as an int64 rather than a uint64, to match the
> BIGINT column used to emit
> queryId in system views
> "
>
> I think this comment is needed to address the confusion that is the
> original subject of this thread. Otherwise,
> the confusion will be moved from pg_stat_statements.c to queryjumblefuncs.c

I ended up adding a comment in the Query struct detailing why queryId
is signed. I imagine, as time passes, we might forget why we did that
as hashed values are more naturally unsigned. I think it's wrong to
add comments in DoJumble() to mention details about what the calling
function does. I think the fact that DoJumble() now returns int64
should be a good enough explanation as to why we want to get the hash
value in signed form.

David