Re: Review of pg_archivecleanup -x option patch - Mailing list pgsql-hackers

From Robert Haas
Subject Re: Review of pg_archivecleanup -x option patch
Date
Msg-id CA+TgmoZDYD_W7K_S1ZuEnqVNOaRWYCX=EETX+R27VB7akrrcEQ@mail.gmail.com
Whole thread Raw
In response to Re: Review of pg_archivecleanup -x option patch  (Jaime Casanova <jaime@2ndquadrant.com>)
Responses Re: Review of pg_archivecleanup -x option patch
List pgsql-hackers
On Fri, Mar 9, 2012 at 12:47 AM, Jaime Casanova <jaime@2ndquadrant.com> wrote:
>> Sorry, here's the patch rebased and with the suggestion from Alex.
>> Which reminds me, I never thank him for the review (shame on me) :D
>
> with the patch this time

This may be a stupid idea, but it seems to me that it might be better
to dispense with all of the logic in here to detect whether the file
name is still going to be long enough after chomping the extension.  I
feel like that's just making things complicated.  I assume the
extensions we're thinking people will want to strip here are things
like ".gz", in which case there should be no confusion; and if
someone's dumb enough to use an extension like "0" (with no dot or
anything) but only sometimes then I think they deserve what they get
(viz: errors).  See attached (only lightly tested) patch for what I'm
thinking of.

Also, I'm wondering about this warning in the documentation:

+        extension added by the compression program.  Note that the
+        <filename>.backup</> file name passed to the program should not
+        include the extension.

IIUC, the latest change you made makes that warning obsolete, no?

[rhaas pgsql]$ contrib/pg_archivecleanup/pg_archivecleanup -d -x .gz .
000000010000000000000010.00000020.backup.gz
pg_archivecleanup: keep WAL file "./000000010000000000000010" and later

Thoughts?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Attachment

pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: pg_prewarm
Next
From: Robert Haas
Date:
Subject: Re: Command Triggers, patch v11