Re: Add comment explaining why queryid is int64 in pg_stat_statements
| От | Lukas Fittl |
|---|---|
| Тема | Re: Add comment explaining why queryid is int64 in pg_stat_statements |
| Дата | |
| Msg-id | CAP53PkzxJB3tREbsJj2jjqT+9c5hSopj92TNV25gRto=f+rkFw@mail.gmail.com обсуждение исходный текст |
| Ответ на | Re: Add comment explaining why queryid is int64 in pg_stat_statements (Michael Paquier <michael@paquier.xyz>) |
| Список | 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);
...
}
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 по дате отправления: