Re: New function pg_stat_statements_reset_query() to reset statistics of a specific query

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, sk(at)zsrv(dot)org, vik(dot)fearing(at)2ndquadrant(dot)com, Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Magnus Hagander <magnus(at)hagander(dot)net>, Robert Haas <robertmhaas(at)gmail(dot)com>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>, Euler Taveira <euler(at)timbira(dot)com(dot)br>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Simon Riggs <simon(at)2ndquadrant(dot)com>, Dave Page <dpage(at)pgadmin(dot)org>
Subject: Re: New function pg_stat_statements_reset_query() to reset statistics of a specific query
Date: 2018-11-29 03:39:48
Message-ID: CAA4eK1Leh5vSh2JQtJOND1j-3GFU0rgQJNz-hcrRMdT_=2H=cA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Nov 29, 2018 at 8:46 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Wed, Nov 28, 2018 at 10:43:35AM -0300, Alvaro Herrera wrote:
> > On 2018-Nov-28, Amit Kapila wrote:
> >> The problem with this idea is that if someone specifies a particular
> >> parameter using query and the query doesn't return any parameters,
> >> then it can lead to inadvertent behavior. For example, if user uses
> >> something like pg_stat_statements_reset(<valid_user_id>,
> >> <valid_db_id>, SELECT s.queryid FROM pg_stat_statements AS s WHERE
> >> s.query = 'SELECT $1 AS "ONE"'); now, if the query doesn't return any
> >> row, we will remove the stats for all queries that belong to
> >> (userid,dbid). It could be surprising for some users, that's why we
> >> came up with option-4 where we keep the default value of parameters as
> >> 0.
> >
> > Right, I think option 4 is a clear improvement over option 1. I can get
> > behind that one. Since not many people care to vote, I think this tips
> > the scales enough to that side.
>
> With option 4 (as defined in [1]), the database ID or the user ID set to
> 0 means that we don't apply any filter on them, which is sensible. Now,
> we have query IDs which can be set to 0, like parseable utility
> statements.
>

I think we ensure that queryId won't be 0 for utility statements.
Check below part of patch:
* For utility statements, we just hash the query string to get an ID.
*/
if (queryId == UINT64CONST(0))
+ {
queryId = pgss_hash_string(query, query_len);

+ /*
+ * If we are unlucky enough to get a hash of zero(invalid), use queryID
+ * as 2 instead, queryID 1 is already in use for normal statements.
+ */
+ if (queryId == UINT64CONST(0))
+ queryId = UINT64CONST(2);
+ }
+

> When a query ID is set to 0, does this mean that all query
> IDs matching 0 are reset? Or does that mean that we don't filter out by
> query ID?
>

It means the later one "We don't apply any filter for queryId.".

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro HORIGUCHI 2018-11-29 04:05:56 Re: pgbench: improve --help and --version parsing
Previous Message Michael Paquier 2018-11-29 03:16:39 Re: New function pg_stat_statements_reset_query() to reset statistics of a specific query