Hi, I just started looking this over a bit. The first thing I noticed is that it adds a dependency on Archive::Tar which isn't already used anywhere else. Did anybody check whether this exists back in 5.8 installations?
Why is "recovery" added to ALWAYS_SUBDIRS in src/test/Makefile instead of to SUBDIRS? Seems a strange choice. Instead of adding print "# Error output: $stderr\n" if $stderr ne ""; to sub psql, I think it would be better to add line separators, which would be clearer if the error output ever turns into a multiline error messages. It would still show as empty if no stderr is produced; so I think something like if ($stderr ne '') { print "#### Begin standard error\n" print $stderr; print "#### End standard error\n"; } or something like that. In my days of Perl, it was starting to become frowned upon to call subroutines without parenthesizing arguments. Is that no longer the case? Because I notice there are many places in this patch and pre- existing that call psql with an argument list without parens. And it's a bit odd because I couldn't find any other subroutine that we're using in that way. In 005_replay_delay there's a 2s delay configured; then we test whether something is replayed in 1s. I hate tests that run for a long time, but is 2s good enough considering that some of our test animals in buildfarm are really slow? -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers