Re: Missing PG_INT32_MIN in numutils.c - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: Missing PG_INT32_MIN in numutils.c
Date
Msg-id CAB7nPqQkgHY3RijKgQiYGfX=_9CL5auUkd05gRhUCsOk56TV7Q@mail.gmail.com
Whole thread Raw
In response to Re: Missing PG_INT32_MIN in numutils.c  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers
On Wed, Apr 13, 2016 at 11:13 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Wed, Apr 13, 2016 at 10:11 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Robert Haas <robertmhaas@gmail.com> writes:
>>> On Wed, Apr 13, 2016 at 9:38 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>>> 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.
>>
>>> 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.
>>
>> I don't care enough to do that either, but I wanted to point out that
>> it's pretty questionable whether this is a stylistic improvement.
>
> Yeah, fair.  I think it depends on whether you think it is more likely
> that people will (a) grep for PG_INT_MIN32 to find places where we do
> overflow handling or (b) observe the close relationship between the
> two constants on adjacent lines.  Probably I should have waited for
> comments before committing, but I figured we wanted to avoid hardcoded
> constants and didn't think much further.

Well, I think that's an improvement as well when looking for places
checking for overflows. And if you revert the patch, you may want to
look as well at pg_lltoa that does the same business with PG_INT64_MIN
and not a hardcoded constant.
-- 
Michael



pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: fd.c: flush data problems on osx
Next
From: Andres Freund
Date:
Subject: Re: fd.c: flush data problems on osx