Re: Add comment explaining why queryid is int64 in pg_stat_statements - Mailing list pgsql-hackers

From Lukas Fittl
Subject Re: Add comment explaining why queryid is int64 in pg_stat_statements
Date
Msg-id CAP53PkzxJB3tREbsJj2jjqT+9c5hSopj92TNV25gRto=f+rkFw@mail.gmail.com
Whole thread Raw
In response to Re: Add comment explaining why queryid is int64 in pg_stat_statements  (Michael Paquier <michael@paquier.xyz>)
List pgsql-hackers
On Mon, May 19, 2025 at 8:12 PM Michael Paquier <michael@paquier.xyz> wrote:
On Tue, May 20, 2025 at 02:03:37PM +1200, David Rowley wrote:
> Aside from the struct field types changing for Query.queryId,
> PlannedStmt.queryId and PgBackendStatus.st_query_id, the
> external-facing changes are limited to:
>
> 1. pgstat_report_query_id() now returns int64 instead of uint64
> 2. pgstat_get_my_query_id()  now returns int64 instead of uint64
> 3. pgstat_report_query_id()'s first input parameter is now int64
>
> If we were to clean this up, then I expect it's fine to wait until
> v19, as it's not really a problem that's new to v18.

Hmm.  For the query ID, that's not new, but for the plan ID it is.  So
it seems to me that there could be also an argument for doing all that
in v18 rather than releasing v18 with the plan ID being unsigned,
keeping a maximum of consistency for both of IDs.  AFAIK, this is
something that Lukas's plan storing extension exposes as an int64
value to the user and the SQL interfaces, even if it's true that we
don't expose it in core, yet.

Yeah, +1 to making this consistent across both query ID and the new plan ID, and changing both to int64 internally seems best.

Just as an example, in my current work-in-progress version of a new pg_stat_plans extension the plan ID gets set like this ([0]):

static void
pgsp_calculate_plan_id(PlannedStmt *result)
{
...
result->planId = HashJumbleState(jstate);
...
}

With HashJumbleState currently returning a uint64.

Similarly, when reading out the planID from backends, we do:

values[4] = UInt64GetDatum(beentry->st_plan_id);

if Postgres 18 released as-is, and 19 changed this, we'd have to carry a version-specific cast from uint64 to int64 in both places. Not a big deal, but certainly nice to not knowingly introduce this confusion for extension development.

As an additional data point, the existing pg_stat_monitor extension uses a uint64 for planID [1], and pg_store_plans uses a uint32 [2]. I'd expect both of these to update to a int64 internally with the new planID being available in 18.

Thanks,
Lukas
--
Lukas Fittl

pgsql-hackers by date:

Previous
From: Michael Paquier
Date:
Subject: Re: Add comment explaining why queryid is int64 in pg_stat_statements
Next
From: David Rowley
Date:
Subject: Re: wrong query results on bf leafhopper