Re: Review of pg_archivecleanup -x option patch

On 02/01/2012 07:53 AM, Alex Shulgin wrote:
> One problem I've found while trying the example workflow is this:
>
> ~/local/postgresql/HEAD$ ./bin/pg_archivecleanup -d -x .gz data/archive/ 000000010000000000000002.00000020.backup.gz
> pg_archivecleanup: invalid filename input
> Try "pg_archivecleanup --help" for more information.
>
> I think we could accept the suffixed WAL filename and strip the suffix;
> another idea is to actually detect the suffix (if any) from the last
> command line argument, thus rendering the -x option unnecessary.

Going full-on automatic with detecting the suffix seems like it might do
the wrong thing when presented with a strange extension. I know a lot
of the time I use pg_archivecleanup is inside scripts that tend to get
setup and then ignored. If one of those has a bug, or unexpected data
appears in the archive, I'd rather see this sort of major, obvious
failure--not an attempt to sort things out that might make a bad decision.

The smaller step of automatically stripping the specified suffix from
the input file name, when it matches the one we've told the program to
expect, seems like a usability improvement unlikely to lead to bad
things though. I've certainly made the mistake you've shown when using
the patched version of the program myself, just didn't think about
catching and correcting it myself before. We can rev this to add that
feature and resubmit easily enough, will turn that around soon.

--
Greg Smith 2ndQuadrant US greg(at)2ndQuadrant(dot)com Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.com