Moving onto the directory archive part of this patch, the feature seems
to work as advertised; here's a quick test case:
createdb pgbench
pgbench -i -s 1 pgbench
pg_dump -F d -f test
pg_restore -k test
pg_restore -l test
createdb copy
pg_restore -d copy test
The copy made that way looked good. There's a good chunk of code in the
patch that revolves around BLOB support. We need to get someone who is
more familiar with those than me to suggest some tests for that part
before this gets committed. If you could suggest how to test that code,
that would be helpful.
There's a number of small things that I'd like to see improved in new
rev of this code
pg_dump: help message for "--file" needs to mention that this is
overloaded to also specify the output directory too.
pg_dump: the documentation for --file should say the directory is
created, and must not exist when you start. The code catches this well,
but that expectation is not clear until you try it.
pg_restore: the help message "check the directory archive" would be
clearer as "check an archive in directory format".
There are some tab vs. space whitespace inconsistencies in the
documentation added.
The comments at the beginning of functions could be more consistent.
Early parts of the code have a header for each function that's
extensive. Maybe even a bit more than needed. I'm not sure why it's
important to document here which of these functions is
optional/mandatory for example, and getting rid of just those would trim
a decent number of lines out of the patch. But then at the end, all of
the new functions added aren't documented at all. Some of those are
near trivial, but it would be better to have at least a small
descriptive header for them.
The comment header at the beginning of pg_backup_directory is a bit
weird. I guess Philip Warner should still be credited as the author of
the code this was based on, but it's a weird seeing a new file
attributed solely to him. Also, there's an XXX in the identification
field there that should be filled in with the file name.
There's your feedback for this round. I hope we'll see an updated patch
from you as part of the next CommitFest.
--
Greg Smith 2ndQuadrant US greg@2ndQuadrant.com Baltimore, MD
PostgreSQL Training, Services and Support www.2ndQuadrant.us
"PostgreSQL 9.0 High Performance": http://www.2ndQuadrant.com/books
