Horton

Re: Add comment explaining why queryid is int64 in pg_stat_statements

wenhui qiu <qiuwenhuifx@gmail.com>

View thread
From: wenhui qiu <qiuwenhuifx@gmail.com>
To: Shaik Mohammad Mujeeb <mujeeb.sk@zohocorp.com>
Cc: 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-17T03:36:19Z
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.

Hi Shaik
> While it's true that no arithmetic or logical operations are performed on
queryid after the assignment, the overflow technically > occurs at the
point of assignment itself. For example, *entry->key.queryid* holds the
value *12747288675711951805* as a
> *uint64*, but after assigning it to *queryid* (which is an *int64*), it
becomes *-5699455397997599811* *due to overflow*.

> This conversion is intentional - most likely to match the *bigint* type
of the *queryid* column in *pg_stat_statements*. However,
> without an explicit comment, this can be misleading. A beginner reading
this might misinterpret it as an unintentional overflow > or bug and raise
unnecessary concerns. Therefore, it’s worth adding a brief comment
clarifying the intent behind this
> assignment.
+1 agree

On Sat, May 17, 2025 at 1:54 AM Shaik Mohammad Mujeeb <
mujeeb.sk@zohocorp.com> wrote:

> Hi Ilia Evdokimov,
>
> While it's true that no arithmetic or logical operations are performed on
> queryid after the assignment, the overflow technically occurs at the
> point of assignment itself. For example, *entry->key.queryid* holds the
> value *12747288675711951805* as a *uint64*, but after assigning it to
> *queryid* (which is an *int64*), it becomes *-5699455397997599811* *due
> to overflow*.
>
> This conversion is intentional - most likely to match the *bigint* type
> of the *queryid* column in *pg_stat_statements*. However, without an
> explicit comment, this can be misleading. A beginner reading this might
> misinterpret it as an unintentional overflow or bug and raise unnecessary
> concerns. Therefore, it’s worth adding a brief comment clarifying the
> intent behind this assignment.
>
>
>
> Thanks & Regards,
> Shaik Mohammad Mujeeb
> Member Technical Staff
> Zoho Corp
>
>
>
>
>
> ---- On Fri, 16 May 2025 15:12:41 +0530 *Ilia Evdokimov
> <ilya.evdokimov@tantorlabs.com <ilya.evdokimov@tantorlabs.com>>* wrote ---
>
>
> On 15.05.2025 10:08, Shaik Mohammad Mujeeb wrote:
>
>
> I don't think the comment is necessary here. There are no arithmetic or
> logical operations performed on it. It is only passed as a Datum.
>
> --
> Best regards,
> Ilia Evdokimov,
> Tantor Labs LLC.
>
> Hi Developers,
>
> In pg_stat_statements.c, the function *pg_stat_statements_internal()*
> declares the *queryid* variable as *int64*, but assigns it a value of
> type *uint64*. At first glance, this might appear to be an overflow
> issue. However, I think this is intentional - the queryid is cast to
> *int64* *to match the bigint type of the queryid column in the
> pg_stat_statements view*.
>
> Please find the attached patch, which adds a clarifying comment to make
> the rationale explicit and avoid potential confusion for future readers.
>
>
>
> Thanks and Regards,
> Shaik Mohammad Mujeeb
> Member Technical Staff
> Zoho Corp
>
>
>
>
>