Re: contrib/pg_stat_statements 1212

On Fri, Dec 12, 2008 at 02:51, ITAGAKI Takahiro
<itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp> wrote:
> Here is an updated version of pg_stat_statements.

I took the liberty of splitting this out into 3 patches for easier review:

patch: 001-hooks.patch
-adds 2 new hooks (startup_hook and shutdown_hook).
-fixes missing process_shared_preload_libraries on EXEC_BACKEND
-changes int pgstat_track_activity_query_size to PGDLLIMPORT int
pgstat_track_activity_query_size
-addes sourceText to QueyDesc and modifies callers to populate it

patch: 002-explain-fmt.patch
-adds new guc explain_analyze_format to print out statistic counters
-adds rtime/utime to explain analyze printout
-moves statistic counters (ReadBufferCount etc...) into struct
Instrumentation via a new struct BufferCounter (allows modules like
the proposed easy access)

patch: pg_stat_statements-1212.patch
the contrib module

A few comments:

Is there a reason you add sourceText to QueryDesc? AFAICT you can do
ActivePortal->sourceText and it will always be populated correctly.

I think the explain_analyze_format guc is a clever way of getting
around the explain analyze verbose you proposed earlier. But I dont
see any doc updates for it.

Im still not overly fond of the "statistics." custom guc name, but
what can you do...

Other than that it looks good, though I admit I have not had the time
to sit down and thoroughly test it yet...