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
