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

On Wed, Jan 9, 2019 at 2:25 PM Amit Kapila <amit.kapila16@gmail.com> wrote:

> Few minor comments:
>

Thanks for the review.


> 1.
> bench=# SELECT query, calls, total_time, rows, 100.0 * shared_blks_hit /
>                nullif(shared_blks_hit + shared_blks_read, 0) AS hit_percent
>           FROM pg_stat_statements ORDER BY total_time DESC LIMIT 5;
>
> -[ RECORD 5
> ]--------------------------------------------------------------------
> query       | vacuum analyze pgbench_accounts
> calls       | 1
> total_time  | 136.448116
> rows        | 0
> hit_percent | 99.9201915403032721
>
> There is nothing between the first and second time you ran this query,
> so where did this vacuum analyze .. record came?
>

Because of the pg_stat_statements_reset() function call to reset the first
query, when the query executed second time the above statement is appeared
as the select query is limiting the result to 5.


> 2.
> bench=# SELECT pg_stat_statements_reset(0,0,s.queryid) FROM
> pg_stat_statements AS s
> bench-# WHERE s.query = 'UPDATE pgbench_branches SET bbalance =
> bbalance + $1 WHERE bid = $2';
>
> bench=#
>
> I think it would be good if you show the output of
> pg_stat_statements_reset statement instead of showing empty line.
>

The pg_stat_statements_reset() function just returns void, because
of this reason, already existing _reset() call also just lists the empty
line, I also followed the same.

I feel it is better to add the return data for all the _reset() function
calls
or leave it as empty.


> Another minor point is that in the second statement
> (pg_stat_statements_reset), you seem to have made it a two-line
> statement whereas first one looks to be a single-line statement, it
> would be good from the readability point of view if both looks same.
> I would prefer the second to look similar to the first one.
>

OK. Corrected.

Updated patch attached.

Regards,
Haribabu Kommi
Fujitsu Australia