Skip site navigation (1) Skip section navigation (2)

Review of pg_archivecleanup -x option patch

From: Alex Shulgin <ash(at)commandprompt(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Review of pg_archivecleanup -x option patch
Date: 2012-02-01 12:53:47
Message-ID: 87obtin290.fsf@commandprompt.com (view raw or flat)
Thread:
Lists: pgsql-hackers

Hello,

This is my first patch review ever, so please bear with me.

The patch[1] is in the context diff format and applies cleanly to
current git HEAD.  Documentation is updated by the patch.

The code does implement what's the patch is supposed to do.

Do we want that?  According to Greg's original mail, yes.

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.

By the way, I for one would use the word 'suffix' instead of 'extension'
which sounds like some MS-DOS thingy, but after briefly grepping the
docs I can see that both words are used in this context already (and the
ratio is not in favor of my choice of wording.)

Other than that the patch looks good.

--
Alex

[1] http://archives.postgresql.org/pgsql-hackers/2011-02/msg00547.php

Responses

pgsql-hackers by date

Next:From: Marko KreenDate: 2012-02-01 12:56:44
Subject: libpq: fix sslcompression leak
Previous:From: Ashutosh BapatDate: 2012-02-01 12:38:06
Subject: Re: Confusing EXPLAIN output in case of inherited tables

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group