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: Peter Eisentraut <peter@eisentraut.org>
Cc: Michael Paquier <michael@paquier.xyz>, Lukas Fittl <lukas@fittl.com>, 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-22T02:36:38Z
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.

On Thu, 22 May 2025 at 02:58, Peter Eisentraut <peter@eisentraut.org> wrote:
>
> On 20.05.25 08:38, Michael Paquier wrote:
> > On Tue, May 20, 2025 at 05:51:51PM +1200, David Rowley wrote:
> >> Given the planId stuff is new and has the same issue, I think that
> >> pushes me towards thinking now is better than later for fixing both.
> >>
> >> I'm happy to adjust my patch unless you've started working on it already.
> >
> > Here you go with the attached, to-be-applied on top of your own patch.
>
> Whichever way we're going, surely this whole thing could benefit from a
>
>      typedef something QueryId;

This part I'm not so sure about. It's not as if it can be changed to
some other type with a simple definition of the typedef.  Looking at
the patch I proposed there are quite a few things that would still
need to be updated after the typedef is changed. PG_GETARG_INT64(),
INT64CONST() and DatumGetInt64() are part of that. There's also the
return type of hash_any_extended().  Changing the typedef definition
might also need an adjustment in gen_node_support.pl, depending on
what you're changing it to.

You could argue that if it reduces the locations that need to be
changed by using a typedef, then it's a win. But there are also
negative aspects to typedefs that need to be considered. For me, those
are the added level of indirection of code reading to actually who
what type I'm working with. I personally dislike typedefs like
"typedef PageHeaderData *PageHeader;" as they hide the fact I'm
working with a pointer.

I'm not outright objecting to the typedef for this. It's just I don't
see it as a clear-cut improvement for this case.

David