Re: New function pg_stat_statements_reset_query() to reset statisticsof a specific query - Mailing list pgsql-hackers

From Haribabu Kommi
Subject Re: New function pg_stat_statements_reset_query() to reset statisticsof a specific query
Date
Msg-id CAJrrPGcCR1AGxb0VSoFwYXi6zNBtbrpRJcV2iY=8WEz0_9t_ag@mail.gmail.com
Whole thread Raw
In response to Re: New function pg_stat_statements_reset_query() to reset statisticsof a specific query  (Amit Kapila <amit.kapila16@gmail.com>)
Responses Re: New function pg_stat_statements_reset_query() to reset statisticsof a specific query  (Amit Kapila <amit.kapila16@gmail.com>)
List pgsql-hackers

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
Attachment

pgsql-hackers by date:

Previous
From: Michael Paquier
Date:
Subject: Re: A few new options for vacuumdb
Next
From: "Higuchi, Daisuke"
Date:
Subject: RE: Problem during Windows service start