On 17.05.25 14:49, Michael Paquier wrote:
> On Fri, May 16, 2025 at 04:05:01PM +0530, Shaik Mohammad Mujeeb wrote:
>> 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.
>
> I don't quite see the value in the comment addition you are suggesting
> here: all the user-facing features related to query IDs use signed 64b
> integers, and are documented as such in the official docs. The fact
> that we store an unsigned value in the backend core code is an
> internal implementation artifact, the important point is that we have
> a value stored in 8 bytes.
There is another, new instance of this confusion. When query IDs are
printed to the log, we use a signed-integer format, but the value is
unsigned (see log_status_format(), write_jsonlog()). This works
correctly in practice, but it triggers warnings from
-Wwarnings-format-signedness (not in the default warnings set, but
useful to clean up occasionally) and maybe other tools along those
lines. This is new because in the past this warning was hidden by
additional casts. If we want to keep this arrangement, maybe we should
create an explicit function to convert query IDs for output, and then
explain all this there. Or why not store query IDs as int64_t
internally, too?