Re: vacuumlo patch

On Tue, Jul 26, 2011 at 12:18 PM, Timothy D. F. Lewis
<elatllat(at)gmail(dot)com> wrote:
> I'm not sure what David did for me so as per Roberts suggestion I have added
> this patch to the commit fest.
> I'm hoping I have not put this patch in more than one workflow.

Hi Tim,

I would be willing to review this patch for the next CommitFest. I'd
like to request that you send an updated version of your patch *as an
attachment* to avoid the problems with long lines getting
automatically wrapped, as Alvaro mentioned. I had trouble getting the
existing patches to apply.

A few preliminary comments about the patch:

1. It wasn't clear to me whether you're OK with Aron's suggested
tweak, please include it in your patch if so.

2. I think it might be better to use INT_MAX instead of hardcoding 2147483647.

3. Your patch has some minor code style differences wrt. the existing
code, e.g.
+ if(param->transaction_limit!=0 &&
deleted>=param->transaction_limit)
should have a space before the first '(' and around the '!=' and '>='

4. The rest of the usage strings spell out 'large object(s)' instead
of abbreviating 'LO'
+ printf(" -l LIMIT stop after removing LIMIT LOs\n");

5. transaction_limit is an int, yet you're using strtol() which
returns long. Maybe you want to use atoi() or make transaction_limit a
long?

Thanks
Josh