Re: Add comment explaining why queryid is int64 in pg_stat_statements

From: wenhui qiu <qiuwenhuifx(at)gmail(dot)com>
To: Shaik Mohammad Mujeeb <mujeeb(dot)sk(at)zohocorp(dot)com>
Cc: 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-17 03:36:19
Message-ID: CAGjGUAJqztUTR=SYLQuS0GjV=C5DEhftp6LAUNPxdY3oopR0ZQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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(dot)sk(at)zohocorp(dot)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(dot)evdokimov(at)tantorlabs(dot)com <ilya(dot)evdokimov(at)tantorlabs(dot)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
>
>
>
>
>

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2025-05-17 12:49:23 Re: Add comment explaining why queryid is int64 in pg_stat_statements
Previous Message Regina Obe 2025-05-16 22:18:52 pg_upgrade ability to create extension from scripts