Re: Review of pg_archivecleanup -x option patch

On Sun, Mar 11, 2012 at 9:28 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Sat, Mar 10, 2012 at 2:38 PM, Jaime Casanova <jaime@2ndquadrant.com> wrote:
>> On Fri, Mar 9, 2012 at 9:07 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>>> 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.
>>
>> while i like the idea of separating the logic, i don't like the results:
>>
>> for example i tried this (notice that i forgot the point), and it just
>> says nothing (not even that the file with the extension but without
>> the point doesn't exist). that's why we were checking that the length
>> matches
>>
>> $ ./pg_archivecleanup -x "bz2" /tmp 000000010000000100000058
>
> Hmm, but I thought that the idea was that the extension was optional.
> Perhaps I'm missing something but I don't think the previous patch
> will complain about that either; or at least I don't see why the
> behavior should be any different.

Can someone enlighten me on this point?

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