Re: Add comment explaining why queryid is int64 in pg_stat_statements

From: David Rowley <dgrowleyml(at)gmail(dot)com>
To: Sami Imseih <samimseih(at)gmail(dot)com>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, Lukas Fittl <lukas(at)fittl(dot)com>, Peter Eisentraut <peter(at)eisentraut(dot)org>, Shaik Mohammad Mujeeb <mujeeb(dot)sk(at)zohocorp(dot)com>, ilyaevdokimov <ilya(dot)evdokimov(at)tantorlabs(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, mujeebskdev <mujeeb(dot)sk(dot)dev(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Add comment explaining why queryid is int64 in pg_stat_statements
Date: 2025-05-30 11:08:58
Message-ID: CAApHDvrEKYXt18rAt6C7mC4cWNTqsE8EXiH7wh7jDwWEXt9Keg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Thanks for the review.

On Wed, 21 May 2025 at 03:35, Sami Imseih <samimseih(at)gmail(dot)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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2025-05-30 11:09:23 Re: Add comment explaining why queryid is int64 in pg_stat_statements
Previous Message Amit Kapila 2025-05-30 11:02:28 Re: Replication slot is not able to sync up