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

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, sk(at)zsrv(dot)org, 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>, Pg 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-09-25 05:09:04
Message-ID: 20180925050904.GI1354@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Sep 25, 2018 at 01:49:09PM +1000, Haribabu Kommi wrote:
> Thanks for the review.
> Fixed in the attached patch as per your suggestion.

Hmm. I see a problem with the tests and the stability of what
pg_stat_statements_reset() can return. Normally installcheck is
disabled in contrib/pg_stat_statements/Makefile but if you remove this
barrier and run the tests with a server loading the module in
shared_preload_libraries then things are not stable. We don't have this
kind of instability on HEAD. Some call to pg_stat_statements_reset()
system-wide is visibly missing.

+ if (!pgss || !pgss_hash)
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("pg_stat_statements must be loaded via shared_preload_libraries")));
This check can be within entry_reset().

+ the specified userid, dbid and queryid. Returns the total number of
+ statement statistics that are reset based on the specified input.
+ If any of the parameter is not specified, the default value NULL(invalid)
Missing some markups for the three field names here, as well as for NULL
which is a value.

I can buy the compatibility breakage with the return result of
pg_stat_statements_reset when specified without arguments.

Some nannyism: If all entries are removed and a new file needs to be
written, you could save a bit of indentation by returning immediately
when (num_entries != num_remove).
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2018-09-25 05:26:42 Re: SSL tests failing with "ee key too small" error on Debian SID
Previous Message Tom Lane 2018-09-25 04:49:47 PG vs macOS Mojave