Hi Sergei, > I still would like to maintaint the focus on committing the "idle in transactions" part of pg_stat_session first.
Agreed. I've done a review of version 0004. This version is applied successful over ce571434ae7, installcheck passed. The behavior of pg_stat_session view and corresponding function looks correct. I've didn't found any issues in the code. Notes about the current state of a patch: Naming the view and function names 'pg_stat_session' seems correct for this particular scope of a patch. However possible future resource consumption statistics are valid for all backends (vacuum for example). Right now it is not clear for me if we can get resource statistics from those backends while those are listed in the pg_stat_activity view but renaming to something like 'pg_stat_backend' seems reasonable to me. Docs 1. session states referenced in monitoring.sgml is not uniform with those of the pg_stat_activity view. monitoring.sgml:4635 monitoring.sgml:4644 + Time in milliseconds this backend spent in the <literal>running</literal> or <literal>fastpath</literal> state. I think those states should be referenced uniformly with pg_stat_activity. 2. Description of the 'pg_stat_get_session()' function might be as follows: Returns a row, showing statistics about the client backend with the specified process ID, or one row per client backend if <literal>NULL</literal> is specified. The fields returned are the same as those of <structname>pg_stat_session</structname> view. The main thought here is to get rid of 'each active backend' because 'active backend' looks like backend in the 'active' state. Tests Call to a non-existing function is depend on non-existence of a function, which can't be guaranteed absolutely. How about to do some kind of obvious error here? Couple examples follows: SELECT 0/0; - or - DO $$ BEGIN RAISE 'test error'; END; $$ LANGUAGE plpgsql; My personal choice would be the last one. -- regards, Andrei Zubkov Postgres Professional
