Re: Missing PG_INT32_MIN in numutils.c

On Wed, Apr 13, 2016 at 9:38 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Wed, Apr 13, 2016 at 3:49 AM, Michael Paquier
>> <michael.paquier@gmail.com> wrote:
>>> While going through numutils.c I found the following thing:
>>> --- a/src/backend/utils/adt/numutils.c
>>> +++ b/src/backend/utils/adt/numutils.c
>>> @@ -136,7 +136,7 @@ pg_ltoa(int32 value, char *a)
>>> * Avoid problems with the most negative integer not being representable
>>> * as a positive integer.
>>> */
>>> -   if (value == (-2147483647 - 1))
>>> +   if (value == PG_INT32_MIN)
>>> {
>>> memcpy(a, "-2147483648", 12);
>>> return;
>>> Attached is a patch. The interesting part is that pg_lltoa is not
>>> missing the check on PG_INT64_MIN.
>
>> Committed.
>
> I am not very convinced that this is an improvement, because you took
> what had been two hard-wired constants and replaced them with a symbol
> and a hard-wired constant.This is more prone to break, not less so.
> If there were a way to stringify PG_INT32_MIN's value for use in the
> memcpy (which would then better be strcpy), then converting *both*
> constants would be an improvement --- but otherwise I think this was
> best left alone.

*shrug*

I think it's kind of six of one, half a dozen of the other, but if you
feel strongly about it, revert the patch.

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