Re: vacuumlo patch

From: Tim <elatllat(at)gmail(dot)com>
To: Josh Kupershmidt <schmiddy(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 04:41:25
Message-ID: CA+3zgms2v7Kinaw9V7=Kjqyx27UBzkrhG-59U_TZRzkYqRfP2g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Josh,

Thanks for help. Attached is a patch including changes suggested in your
comments.

Excerpts from Josh's message On Sat, Aug 6, 2011 at 9:57 PM:

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

I decided to and included Aron's tweak.
I'm not sure if the PostgreSQL project prefers simplified code over faster
run time (if only under rare conditions).
Who knows maybe the compiler will re-optimize it anyway.

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

Fixed, though I'm not sure if I put the include in the preferred order.

> 3. ... should have a space before the first '(' and around the '!=' and
> '>='
>

Fixed.

> 4. The rest of the usage strings spell out 'large object(s)' instead
> of abbreviating 'LO'...
>

Fixed, I omitted the brackets around the s to conform with the other usage
strings.

> 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?
>

The other INT in the code is using strtol() so I also used strtol instead of
changing more code.
I'm not sure if there are any advantages or disadvantages.
maybe it's easier to prevent/detect/report overflow wraparound.

I can't think of a good reason anyone would want to limit transaction to
something more than INT_MAX.
Implementing that would best be done in separate and large patch as
PQntuples returns an int and is used on 271 lines across PostgreSQL.

Thanks again for the help.
<2147483647>

Attachment Content-Type Size
vacuumlo.patch application/octet-stream 2.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Josh Kupershmidt 2011-08-07 06:36:41 Re: vacuumlo patch
Previous Message Josh Kupershmidt 2011-08-07 01:57:47 Re: vacuumlo patch