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
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 |