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

On Tue, Sep 25, 2018 at 3:09 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:

> 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.
>

Thanks for the review.

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.
>

The difference in results of the output of the pg_stat_statements_reset()
function is based on the earlier statements that are stored in the
pg_stat_statements
table, this varies based on the environment. So I created a wrapper function
that masks the return value of the first reset and then the test is stable.

check whether is it fine or any better approach to handle it?

> + 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().
>

Moved.

> + 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.
>

Corrected.

> 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).
>

Corrected.

Attached is the updated patch.

Regards,
Haribabu Kommi
Fujitsu Australia