This review was compiled from a PDXPUG group review (Dan Colish, Mark Wong, Brent Dombrowski, and Gabrielle Roth).
-- We all agree this is a really useful feature. The patch applies cleanly to the current git master with git apply, it's in context diff, and does what it's supposed to do on Ubuntu, OSX, and gentoo. We found a few problems with it, and we'd like to see the patch resubmitted with the following changes: Tests: - Regression test requires plpythonu; it needs to work without that. Docs: - The doc comment 'pgstat_get_backend_current_activity' doesn't match the function name 'pgstat_get_crashed_backend_activity'. Project coding guidelines: - There are some formatting problems, such as all newlines at the same indent level need to line up. (The author mentioned "not [being] happy with the indenting depth", so I think this is not a surprise.) - Wayward tabs, line 2725 in pgstat.c specifically - Unknown is used a lot (see http://developer.postgresql.org/pgdocs/postgres/error-style-guide.html#AEN94099) We had some questions about ascii_safe_strncpy: - It doesn't convert just unknown encodings, it converts all encodings. Is that intentional? - Is there an existing function that already does this? Looking forward to the next revision! gabrielle (on behalf of PDXPUG) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers