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

From: Arthur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>
To: Julian Markwort <julian(dot)markwort(at)uni-muenster(dot)de>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andres Freund <andres(at)anarazel(dot)de>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Simon Riggs <simon(at)2ndquadrant(dot)com>, David Steele <david(at)pgmasters(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, marius(dot)timmer(at)uni-muenster(dot)de, arne(dot)scheffer(at)uni-muenster(dot)de
Subject: Re: [HACKERS] [FEATURE PATCH] pg_stat_statements with plans (v02)
Date: 2018-03-13 16:29:25
Message-ID: 20180313162924.GA22449@zakirov.localdomain
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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:
Russian Postgres Company

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message David Steele 2018-03-13 16:34:16 Re: PATCH: Exclude temp relations from base backup
Previous Message Mat Arye 2018-03-13 16:20:27 Re: Additional Statistics Hooks