Yuya Watari <watari.yuya@gmail.com> writes:
> On Thu, Nov 7, 2019 at 3:10 PM Kyotaro Horiguchi
> <horikyota.ntt@gmail.com> wrote:
>> + if (unlikely(!FLOAT8_FITS_IN_INT32(num)) || isnan(num))
>> If compiler doesn't any fancy, num is fed to an arithmetic before
>> checking if it is NaN. That seems have a chance of exception.
> Thank you for pointing it out. That's my mistake. I fixed it and
> attached the patch.
Actually, that mistake is very old --- the existing functions tested
isnan() last for a long time. I agree that testing isnan() first
is safer, but it seems that the behavior of throwing an exception
for comparisons on NaN is rarer than one might guess from the C spec.
Another issue in the patch as it stands is that the FITS_IN_ macros
require the input to have already been rounded with rint(), else they'll
give the wrong answer for values just a bit smaller than -PG_INTnn_MIN.
The existing uses of the technique did that, and interval_mul already
did too, but I had to adjust pgbench. This is largely a documentation
failure: not only did you fail to add any commentary about the new macros,
but you removed most of the commentary that had been in-line in the
existing usages.
I fixed those things and pushed it.
regards, tom lane