Re: vacuumlo patch

On Tue, Jul 26, 2011 at 12:18 PM, Timothy D. F. Lewis
<elatllat@gmail.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