On Tue, Dec 2, 2008 at 02:35, ITAGAKI Takahiro <[EMAIL PROTECTED]> 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.
*** a/contrib/pg_stat_statements/pg_stat_statements.c --- b/contrib/pg_stat_statements/pg_stat_statements.c *************** *** 216,222 **** error: } /* ! * pgss_shutdown - Load statistics from file. */ static void pgss_startup(void) --- 216,222 ---- } /* ! * pgss_startup - Load statistics from file. */ static void pgss_startup(void) *** a/doc/src/sgml/pgstatstatements.sgml --- b/doc/src/sgml/pgstatstatements.sgml *************** *** 68,74 **** <entry><structfield>calls</structfield></entry> <entry><type>bigint</type></entry> <entry></entry> ! <entry>Number of being executed</entry> </row> <row> --- 68,74 ---- <entry><structfield>calls</structfield></entry> <entry><type>bigint</type></entry> <entry></entry> ! <entry>Number of times executed</entry> </row> <row>
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers