Re: track generic and custom plans in pg_stat_statements

> > I know Michael opposed the idea of  carrying these structures,
> > at least CachedPlan, to Executor hooks ( or maybe just not QueryDesc?? ).
> > It will be good to see what he think, or if others an opinion about this,
> > about
> > adding a pointer to CachedPlanSource in PlannedStmt vs setting a flag in
> > PlannedStmt to track plan cache type for the current execution? The
> former
> > does provide more capability for extensions, as Andrei has pointed out
> > earlier.
>
> My line is pretty simple here: we should never include in
> executor-specific or plan-specific areas structures that are handled
> by entirely different memory contexts or portions of the code, all
> parts (executor, query description and or [cached] plan) relying on
> entirely different assumptions and contexts, because I suspect that it
> would bite us back hard in the shape of corrupted stacks depending on
> the timing where these resources are freed.  One of the upthread
> proposals where a reference of the cached plan pointer was added to an
> executor state structure crossed this line.
>
> Simple fields are no-brainers, they are just carried in the same
> context as the caller, independently of other states.  When it comes
> to stats and monitoring, we don't have to be exact, we can sometimes
> even be a bit lossy in the reports.
>
> A simple "plan type" field in a PlannedStmt should be more than OK, as
> it would be carried across the contexts where we want to know about it
> and handle it, aka PGSS entry storing.
>

Thanks!

You make valid point about memory contexts and opening ourselves to
bugs/crashes.

[0] implements a single field “plan type” in PlannedStmt.

[0]
https://www.postgresql.org/message-id/CAA5RZ0tA43qf1HF1g6oYUGNBGFr23F29pf4F9Hymp2z8PyTsPg%40mail.gmail.com

--

Sami