On Mon, May 19, 2025 at 8:12 PM Michael Paquier <mich...@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
[0]:
https://github.com/pganalyze/pg_stat_plans/blob/main/pg_stat_plans.c#L745
[1]:
https://github.com/percona/pg_stat_monitor/blob/9c72c2e73d83a6b687ff8e2ddc5072c63afe983b/pg_stat_monitor.h#L212
[2]:
https://github.com/ossc-db/pg_store_plans/blob/master/pg_store_plans.c#L129
--
Lukas Fittl
