From: | Josh Kupershmidt <schmiddy(at)gmail(dot)com> |
---|---|
To: | "Timothy D(dot) F(dot) Lewis" <elatllat(at)gmail(dot)com> |
Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, David Fetter <david(at)fetter(dot)org>, Aron Wieck <me(at)eunice(dot)de> |
Subject: | Re: vacuumlo patch |
Date: | 2011-08-07 01:57:47 |
Message-ID: | CAK3UJRHtKq01VO-g1YJg=HqA4NDd3Naajr1v+F9=UZ_xGz6zcQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Tim | 2011-08-07 04:41:25 | Re: vacuumlo patch |
Previous Message | Dimitri Fontaine | 2011-08-06 20:24:39 | Re: Transient plans versus the SPI API |