Re: Add comment explaining why queryid is int64 in pg_stat_statements

Sami Imseih <samimseih@gmail.com>

From: Sami Imseih <samimseih@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: David Rowley <dgrowleyml@gmail.com>, 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-20T15:35:39Z
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.

Supporting unsigned INTs will be valuable for more than just this
obviously, and if we do
ever have that capability, we would likely want to go that route. I
know I've been asked about
why queryIds could be negative more than a few times. Until then, I
think the patch being
suggested is reasonable and cleaner.

A few comments on the patches:

1/ this was missed in pg_stat_statements
./contrib/pg_stat_statements/pg_stat_statements.c:    uint64
saved_queryId = pstmt->queryId;

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

DoJumble(JumbleState *jstate, Node *node)
{
/* Jumble the given node */
@@ -208,9 +208,9 @@ DoJumble(JumbleState *jstate, Node *node)
FlushPendingNulls(jstate);
/* Process the jumble buffer and produce the hash value */
- return DatumGetUInt64(hash_any_extended(jstate->jumble,
- jstate->jumble_len,
- 0));
+ return DatumGetInt64(hash_any_extended(jstate->jumble,
+ jstate->jumble_len,
+ 0));
}
/*
@@ -256,10 +256,10 @@ AppendJumbleInternal(JumbleState *jstate, const
unsigned char *item,
if (unlikely(jumble_len >= JUMBLE_SIZE))
{
- uint64 start_hash;
+ int64 start_hash;
- start_hash = DatumGetUInt64(hash_any_extended(jumble,
- JUMBLE_SIZE, 0));
+ start_hash = DatumGetInt64(hash_any_extended(jumble,
+ JUMBLE_SIZE, 0));
memcpy(jumble, &start_hash, sizeof(start_hash));
jumble_len = sizeof(start_hash);

--
Sami Imseih
Amazon Web Services (AWS)