Re: contrib/pg_stat_statements 1202 - Mailing list pgsql-hackers

From Alex Hunsaker
Subject Re: contrib/pg_stat_statements 1202
Date
Msg-id 34d269d40812071813v281326c8s47fd40b0dfddd2f4@mail.gmail.com
Whole thread Raw
In response to contrib/pg_stat_statements 1202  (ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>)
Responses Re: contrib/pg_stat_statements 1202
Re: contrib/pg_stat_statements 1202
Re: contrib/pg_stat_statements 1202
List pgsql-hackers
On Tue, Dec 2, 2008 at 02:35, ITAGAKI Takahiro
<itagaki.takahiro@oss.ntt.co.jp> wrote:
> Here is an update version of contrib/pg_stat_statements.

Hello again!

I was assigned to review this.

Submission review:
Is the patch in standard form? Yes
Does it apply cleanly to current HEAD? Yes (with fuzz)
Does it include reasonable tests, docs, etc? Yes

Usability review:
Does the patch actually implement that? Yes
Do we want that? I think so
Do we already have it? No
Does it follow SQL spec, or the community-agreed behavior? Sure
Are there dangers? No
Have all the bases been covered? Yes

Feature test:
Does the feature work as advertised? Yes
Are there corner cases the author has failed to consider? No

Performance review
Does the patch slow down simple tests?

Does not seem to...

(test.sql)
select * from tenk1 a join tenk1 b using (unique1);

(dual core machine, --enable-debug, --enable-cassert build)
pgbench -c 2 -T60 -n -f test.sql

HEAD: tps = 9.674423
PATCH: tps = 9.695784

If it claims to improve performance, does it?
Does it slow down other things?

Coding review:
Does it follow the project coding guidelines? Yes
Are there portability issues? No
Will it work on Windows/BSD etc? Think so
Are the comments sufficient and accurate? I think so
Does it do what it says, correctly? Yes
Does it produce compiler warnings? No
Can you make it crash? No

I'm not sure about the new counters in struct Instrumentation or the
hooks (but did not see anything obviously wrong with them)... A
commiter can better comment on those.  Also find attached some very
minor verbiage changes.  If there is nothing else on your todo list
for this Ill mark it as Ready for commiter on the wiki.

Attachment

pgsql-hackers by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: Updates of SE-PostgreSQL 8.4devel patches (r1268)
Next
From: Tom Lane
Date:
Subject: Re: cvs head initdb hangs on unixware