Re: [HACKERS] [FEATURE PATCH] pg_stat_statements with plans (v02)

On Mon, Mar 12, 2018 at 02:11:42PM +0100, Julian Markwort wrote:
> > In same manner pg_stat_statements_good_plan_reset() and
> > pg_stat_statements_bad_plan_reset() functions can be combined in
> > function:
>
> > pg_stat_statements_plan_reset(in queryid bigint, in good boolean, in bad
> > boolean)
>
> This is also sensible, however if any more kinds of plans would be added in the future, there would be a lot of flags in this function. I think having varying amounts of flags (between extension versions) as arguments to the function also makes any upgrade paths difficult. Thinking more about this, we could also user function overloading to avoid this.
> An alternative would be to have the caller pass the type of plan he wants to reset. Omitting the type would result in the deletion of all plans, maybe?
> pg_stat_statements_plan_reset(in queryid bigint, in plan_type text)

Yes, it is a good idea. But maybe instead of passing an empty string
into plans type we should overload the function with only queryid
argument. I think it is a common practice in PostgreSQL. Otherwise
allowance to pass empty string as plan type may confuse users. So
functions declaration may be the following:

pg_stat_statements_plan_reset(in queryid bigint, in plan_type text)
pg_stat_statements_plan_reset(in queryid bigint)

+ interquartile_dist = 2.0*(0.6745 * sqrt(e->counters.sum_var_time / e->counters.calls));

I think it would be better to have defines for 2.0 and 0.6745 values for
the sake of readability.

--
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company