Re: track generic and custom plans in pg_stat_statements - Mailing list pgsql-hackers

From Sami Imseih
Subject Re: track generic and custom plans in pg_stat_statements
Date
Msg-id CAA5RZ0ss-jmqeze0G8anztzLRJuMEgz9y3gTd-rgTc2Ejtwzvg@mail.gmail.com
Whole thread Raw
In response to Re: track generic and custom plans in pg_stat_statements  (Andrei Lepikhov <lepihov@gmail.com>)
Responses Re: track generic and custom plans in pg_stat_statements
List pgsql-hackers
> > "plan cache mode" to be accessible in ExecutorEnd somehow.
> I agree with this - adding references to CachedPlan into the QueryDesc
> looks kludge.
> The most boring aspect of pg_stat_statements for me is the multiple
> statements case: a single incoming query (the only case in the cache of
> a generic plan) may be rewritten as various statements with the same
> query ID. So, the execution time of the initial statement should be the
> sum of the executions of all rewritten parts.

AFAICT, It does sum up the stats of rewritten statements of a single statement,
since each one of those statements does call ExecutorEnd ( under the same
queryId ).

> If the multiple-statements case didn't exist for a cached statement,
> Michael's idea with an active portal would be much better, and it would
> make sense to add PlanCacheSource::cplan and replace Portal::CachedPlan
> with Portal::PlanCacheSource reference.

I don't like ActivePortal because cplan itself does not live by ExecutorEnd [0]
and in some cases it's not available in ExecutorStart ( think of
Explain/Explain Analyze),
so you still need to add some handling for those cases, which I don't
think we can
ignore. Also, if we say we can forgo tracking the Explain/Explain Analyze case,
we surely don't want to call pgss_store at ExecutorStart to set the plan cache
mode stats and ExecutorEnd to set everything else for a particular execution.
In short, we still need some fields to be added to QueryDesc to track the
plan cache mode info.

Last week I published a v11 that adds a field to QueryDesc, but as I thought
about this more, how about we just add 2 bool fields in QueryDesc->estate
( es_cached_plan and es_is_generic_plan ), a field in CachedPlan (
is_generic_plan )
and after ExecutorStart, and if we have a cplan, we set the
appropriate plan cache
mode value. I think it's better to confine these new fields in Estate
rather than
at a higher level like QueryDesc. See v12.

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

--

Sami

Attachment

pgsql-hackers by date:

Previous
From: "Matheus Alcantara"
Date:
Subject: Re: Proposal: QUALIFY clause
Next
From: Vik Fearing
Date:
Subject: Re: Proposal: QUALIFY clause