From: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com> |
---|---|
To: | pgsql-kr(at)postgresql(dot)kr |
Cc: | PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Request improve pg_stat_statements module |
Date: | 2014-02-28 13:01:22 |
Message-ID: | CAB7nPqSJugciZmOAcaS95JX8c1Dv41R9kfsDrGzWdw+pdEPRXg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Thanks for your patch!
On Fri, Feb 28, 2014 at 4:18 PM, <pgsql-kr(at)postgresql(dot)kr> wrote:
> I patched to add one column in pg_stat_statements module.
> and sent to author but
> I need a last time of query, because I want to analyse order by recent time.
Hm... I am not sure that this brings much to pg_stat_statements, which
is interested to gather normalized information about the queries run
on the server. For example, things like calculating the average time
of a query by using total_time/calls or even the total time to guess
where is an application bottleneck is interesting on a busy system,
while the latest timestamp is not really an information that can be
used for query tuning. Getting the latest timestamp when a query has
run is particularly not interesting on OLTP-like applications where
many short transactions are running. It is more interesting to
classify query results by either calls, total_time or the combination
of both IMO.
> this patch code below, review please and
> I wish to apply at next version.
When submitting patches, there are a couple of things to know, they
are summarized here.
https://wiki.postgresql.org/wiki/Submitting_a_Patch
One of the things that would be interesting for future contributions
in your case is knowing the code convention used in Postgres code (see
comments below for more details):
http://www.postgresql.org/docs/devel/static/source.html
> - if (tupdesc->natts == PG_STAT_STATEMENTS_COLS_V1_0)
> + if (tupdesc->natts == PG_STAT_STATEMENTS_COLS_V1_0){
> sql_supports_v1_1_counters = false;
> + sql_supports_v1_2_counters = false;
> + }
This is incorrect style, please create a new like for a bracket, like that:
if (cond)
{
blabla;
blabla2;
}
> per_query_ctx = rsinfo->econtext->ecxt_per_query_memory;
> oldcontext = MemoryContextSwitchTo(per_query_ctx);
> @@ -1185,8 +1195,15 @@
> values[i++] = Float8GetDatumFast(tmp.blk_read_time);
> values[i++] = Float8GetDatumFast(tmp.blk_write_time);
> }
> + // last_executed_timestamp
Please do not use double-slashes for comments, write them instead like that:
/* my comment is here */
Finally be sure to attach a patch to an email and avoid to simply
copy/paste the content of the patch in your email. This is more
user-friendly :) Context diffs are recommended as well.
Regards,
--
Michael
From | Date | Subject | |
---|---|---|---|
Next Message | Thom Brown | 2014-02-28 13:02:57 | Re: jsonb and nested hstore |
Previous Message | Andrew Dunstan | 2014-02-28 13:01:08 | Re: jsonb and nested hstore |