Re: Missing PG_INT32_MIN in numutils.c

On Wed, Apr 13, 2016 at 11:13 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Wed, Apr 13, 2016 at 10:11 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>>> On Wed, Apr 13, 2016 at 9:38 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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