On 3/22/25 06:48, Michael Paquier wrote:
> On Fri, Mar 21, 2025 at 09:25:24AM -0400, Sami Imseih wrote:
>>> planner() is the sole place in the core code where the planner hook
>>> can be called. Shouldn't we have at least a call to
>>> pgstat_report_plan_id() after planning a query? At least that should
>>> be the behavior I'd expect, where a module pushes a planId to a
>>> PlannedStmt, then core publishes it to the backend entry in non-force
>>> mode.
>>
>> I agree. I was just thinking we rely on the exec_ routines to report the plan_id
>> at the start. But, besides the good reason you give, reporting
>> (slightly) earlier is
>> better for monitoring tools; as it reduces the likelihood they find an empty
>> plan_id.
>
> Yep. pgstat_report_plan_id() is not something that extensions should
> do, but they should report the planId in the PlannedStmt and let the
> backend do the rest.
>
>> Overall, v3 LGTM
>
> Thanks. If anybody has any objections and/or comments, now would be a
> good time. I'll revisit this thread at the beginning of next week.
The last time I wrote the email, I mistakenly thought about
plan_node_id. I apologize for that.
planId actually looks less controversial than queryId or plan_node_id.
At the same time, it is not free from the different logic that
extensions may incorporate into this value: I can imagine, for example,
the attempt of uniquely numbering plans related to the same queryId,
plain hashing of the plan tree, hashing without constants, etc.
So, it seems that extensions may conflict with the same field. Are we
sure that will happen if they are loaded in different orders in
neighbouring backends?
I'm not very good at stat collector guts, but would it be possible to
allow extensions to report their state in standard tuple format? In that
case, doubts about queryId or planId may be resolved.
--
regards, Andrei Lepikhov
