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
